diff --git a/.agents/notes/implemented/2026-10-27-the-lease-lets-the-request-finish.md b/.agents/notes/implemented/2026-10-27-the-lease-lets-the-request-finish.md new file mode 100644 index 0000000..4356225 --- /dev/null +++ b/.agents/notes/implemented/2026-10-27-the-lease-lets-the-request-finish.md @@ -0,0 +1,35 @@ +--- +title: The lease lets the request finish, a replay of a handled request is a read, and the lease length is a setting +status: implemented +date: 2026-10-27 +scope: hookrelay +--- + +## Decision + +Three follow-ups to "Recovery must not claim certainty", named in its review. + +1. **A failed renewal no longer cancels the request.** The heartbeat logs and + keeps trying; `complete_signature` decides at the end: a lease still the + request's completes it, a lease lost answers 503. +2. **A replay of a handled request is answered from a read.** The spent row is + looked up on the read connection before the write lock is taken; only a + signature not yet spent takes the write path. The storm fuse is unchanged. +3. **The lease length is `HOOKRELAY_SIGNATURE_LEASE_SECONDS`**, 120 by default + and at least 10, renewed every third of it. + +## Why + +Cancelling mid-pipeline left the sender a dropped connection instead of an +answer and the pipeline half-run, while the completion check already re-asks +whether the lease is still the request's. Every copy of a bulk replay took the +write lock, where the door before the lease answered it from a read. A fixed +120 seconds could not follow a pipeline slower or faster than the one it was +measured on. + +## Consequences + +- A request whose renewals fail still completes while its lease is unexpired; + one that outlives a lost lease answers 503, and the sender's retry decides. +- A replay storm of one capture costs reads, not writes. +- The pipe's ceiling moves 6,650 to 6,700, argued in `scripts/assert_weight.py`. diff --git a/README.md b/README.md index b28dd72..04e7f52 100644 --- a/README.md +++ b/README.md @@ -64,6 +64,6 @@ The same pipe also carries an operator's own work signals to a planner and, on a ## Developing -`bash scripts/gate.sh` runs every service's gate plus the stack checks. Read its verdict before you commit. The pipe caps itself at 6,650 source lines and the judge at 3,450; the investigator is uncapped. +`bash scripts/gate.sh` runs every service's gate plus the stack checks. Read its verdict before you commit. The pipe caps itself at 6,700 source lines and the judge at 3,700; the investigator is uncapped. MIT licensed. diff --git a/deploy/docker-compose.shadow.yml b/deploy/docker-compose.shadow.yml index f3c558f..1c38c4f 100644 --- a/deploy/docker-compose.shadow.yml +++ b/deploy/docker-compose.shadow.yml @@ -41,6 +41,7 @@ services: HOOKRELAY_CONFIG: /etc/hookrelay/shadow.yaml HOOKRELAY_WORKER_INTERVAL: "0.5" HOOKRELAY_SIGNATURE_REPLAY_SECONDS: ${HOOKRELAY_SIGNATURE_REPLAY_SECONDS:-300} # closes the captured-request replay window (0 = bounded only) + HOOKRELAY_SIGNATURE_LEASE_SECONDS: ${HOOKRELAY_SIGNATURE_LEASE_SECONDS:-120} # how long an in-flight signed request holds its signature WW_RELAY_SECRET: ${WW_RELAY_SECRET:?mirror FORWARD_RELAY_SECRET into .env} # The platform's report intake (its /v1/deep-analyses/intake), and the # secret it verifies — mirror of its DEEP_ANALYSIS_REPORT_INTAKE_SECRET. diff --git a/deploy/docker-compose.work.yml b/deploy/docker-compose.work.yml index c985f29..13fe7ed 100644 --- a/deploy/docker-compose.work.yml +++ b/deploy/docker-compose.work.yml @@ -61,6 +61,7 @@ services: HOOKRELAY_CONFIG: /etc/hookrelay/work.yaml HOOKRELAY_WORKER_INTERVAL: "0.5" HOOKRELAY_SIGNATURE_REPLAY_SECONDS: ${HOOKRELAY_SIGNATURE_REPLAY_SECONDS:-300} # closes the captured-request replay window (0 = bounded only) + HOOKRELAY_SIGNATURE_LEASE_SECONDS: ${HOOKRELAY_SIGNATURE_LEASE_SECONDS:-120} # how long an in-flight signed request holds its signature # work.yaml's ${...} refs are expanded by hookrelay from its OWN # environment, not by compose — every name used there has to be passed # through here or it silently resolves to empty. An empty URL crashes at diff --git a/docker-compose.yml b/docker-compose.yml index a427479..fb4e026 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -29,6 +29,7 @@ services: HOOKRELAY_CONFIG: /etc/hookrelay/config.yaml HOOKRELAY_WORKER_INTERVAL: "0.5" HOOKRELAY_SIGNATURE_REPLAY_SECONDS: ${HOOKRELAY_SIGNATURE_REPLAY_SECONDS:-300} # closes the captured-request replay window (0 = bounded only) + HOOKRELAY_SIGNATURE_LEASE_SECONDS: ${HOOKRELAY_SIGNATURE_LEASE_SECONDS:-120} # how long an in-flight signed request holds its signature # The shipped example plugins, mounted read-only below: the DingTalk # dialect the stack config uses is one of them (chat_markdown_channels.py). HOOKRELAY_PLUGINS: /etc/hookrelay/plugins diff --git a/docs/evidence.md b/docs/evidence.md index eae069b..62f311b 100644 --- a/docs/evidence.md +++ b/docs/evidence.md @@ -21,7 +21,7 @@ run. | claim | stated in | artefact that backs it | class | | --- | --- | --- | --- | -| hookrelay stays at or under 6,650 source lines | `hookrelay/README.md` | `scripts/assert_weight.py` | Reproducible | +| hookrelay stays at or under 6,700 source lines | `hookrelay/README.md` | `scripts/assert_weight.py` | Reproducible | | hookjudge stays at or under 3,700 source lines | `hookjudge/README.md` | `scripts/assert_weight.py` | Reproducible | | every route, knob and containment row is written up or named as deliberately not | `docs/containment.md`, `*/docs/reference.md` | `scripts/assert_docs.py` | Reproducible | | the three boards share one design language | the service pages | `scripts/assert_design.py` | Reproducible | diff --git a/hookrelay/README.md b/hookrelay/README.md index db9c04c..073e101 100644 --- a/hookrelay/README.md +++ b/hookrelay/README.md @@ -8,12 +8,12 @@ workflow are its own. Receive webhooks. Decide. Fan out to channels. Nothing else. -A pluggable router (under 6,650 source lines, five dependencies) that takes JSON +A pluggable router (under 6,700 source lines, five dependencies) that takes JSON webhooks in at one door, walks each event through three named gates, and delivers to a chat bridge / generic HTTP (DingTalk and WeCom as shipped plugins) — with retries, per-channel rate limits, and a dead-letter queue you can see. -Both numbers are **budgets, not descriptions**. 6,650 source lines is the +Both numbers are **budgets, not descriptions**. 6,700 source lines is the ceiling and five dependencies is the count; `scripts/assert_weight.py` enforces the first alongside the other stack checks, and crossing it is meant to cost a conversation rather than a commit. Tests are counted and printed but never capped diff --git a/hookrelay/docs/reference.md b/hookrelay/docs/reference.md index 162d7ab..c18b7f4 100644 --- a/hookrelay/docs/reference.md +++ b/hookrelay/docs/reference.md @@ -7,7 +7,7 @@ routes and their first sentence from the handlers. Correct it by editing the comment beside the field or the handler's docstring — an edit here is lost on the next run, and `scripts/gen_reference.py --check` will say so. -## Environment (21) +## Environment (22) | variable | default | what it is | | --- | --- | --- | @@ -27,6 +27,7 @@ the next run, and `scripts/gen_reference.py --check` will say so. | `HOOKRELAY_ACTION_SECRET` | *(empty)* | Signs the buttons on a notification card and verifies them on the way back | | `HOOKRELAY_ACTION_TTL_SECONDS` | `24 * 3600` | How long a card's buttons stay pressable | | `HOOKRELAY_CARD_CALLBACK_SECRET` | *(empty)* | Optional second layer on /card-action: when set, the callback must ALSO carry the family's timestamped signature | +| `HOOKRELAY_SIGNATURE_LEASE_SECONDS` | `120` | How long a request holds its signature's lease while it runs, renewed every third of it; an identical request meanwhile is answered 503 | | `HOOKRELAY_WORKER_INTERVAL` | `1.0` | Delivery-loop tick, in seconds | | `HOOKRELAY_PUBLIC_URL` | *(empty)* | Where a card's action LINK should point | | `HOOKRELAY_UI_LINKS` | *(empty)* | door=url pairs: where the board that owns a door's sessions lives, as a BROWSER reaches it, so a chain's hop can link to the investigation it names | diff --git a/hookrelay/hookrelay/app.py b/hookrelay/hookrelay/app.py index e2a7e84..618c24a 100644 --- a/hookrelay/hookrelay/app.py +++ b/hookrelay/hookrelay/app.py @@ -464,7 +464,7 @@ async def hook(source_name: str, request: Request) -> JSONResponse: # is a refusal to guess rather than a miss. replay: str | None = None owner = uuid.uuid4().hex - lease_seconds = 120 + lease_seconds = app_settings.signature_lease_seconds heartbeat: asyncio.Task[None] | None = None if app_settings.signature_replay_seconds > 0: replay = replay_token(source.name, headers.get("x-hook-signature")) @@ -477,21 +477,18 @@ async def hook(source_name: str, request: Request) -> JSONResponse: if state == "busy": return JSONResponse({"detail": "identical request is still processing"}, 503, {"Retry-After": "1"}) - processing = asyncio.current_task() - async def renew() -> None: + # A failed renewal does not stop the request: cancelling it left + # the sender a dropped connection and the pipeline half-run. + # complete_signature decides below, 503 if the lease is gone. while True: await asyncio.sleep(lease_seconds / 3) try: - renewed = await app.state.store.renew_signature(replay, owner, now_ts(), lease_seconds) + if not await app.state.store.renew_signature(replay, owner, now_ts(), lease_seconds): + logger.error("signature processing lease was lost") + return except Exception: logger.exception("signature processing lease renewal failed") - renewed = False - if not renewed: - logger.error("signature processing lease was lost") - if processing is not None: - processing.cancel() - return heartbeat = asyncio.create_task(renew()) try: diff --git a/hookrelay/hookrelay/settings.py b/hookrelay/hookrelay/settings.py index fe61f39..caa31fd 100644 --- a/hookrelay/hookrelay/settings.py +++ b/hookrelay/hookrelay/settings.py @@ -56,6 +56,11 @@ class Settings: # leaves the window BOUNDED rather than closed, which is what the pipe did # before 2026-10-12 — see security.verify_signature for what that costs. signature_replay_seconds: int = 300 + # How long a request holds its signature's lease while it runs, renewed every + # third of it; an identical request meanwhile is answered 503. Long enough for + # the slowest pipeline answer, short enough that a crashed holder does not keep + # the sender's retry out for long. At least 10. + signature_lease_seconds: int = 120 # Signs the buttons on a notification card and verifies them on the way # back. Empty = no card carries an action and /card-action refuses # everything, which is the right default: an unsigned button is a URL @@ -90,6 +95,7 @@ def load(cls) -> Settings: read_token=os.environ.get("HOOKRELAY_READ_TOKEN", ""), max_body_bytes=_int("HOOKRELAY_MAX_BODY_BYTES", 256 * 1024), signature_replay_seconds=_int("HOOKRELAY_SIGNATURE_REPLAY_SECONDS", 300), + signature_lease_seconds=max(10, _int("HOOKRELAY_SIGNATURE_LEASE_SECONDS", 120)), max_attempts=_int("HOOKRELAY_MAX_ATTEMPTS", 8), retention_days=_int("HOOKRELAY_RETENTION_DAYS", 14), alarm_url=os.environ.get("HOOKRELAY_ALARM_URL", ""), diff --git a/hookrelay/hookrelay/store.py b/hookrelay/hookrelay/store.py index 8e5dcbc..c1fae65 100644 --- a/hookrelay/hookrelay/store.py +++ b/hookrelay/hookrelay/store.py @@ -983,7 +983,13 @@ async def acquire_signature(self, token: str, owner: str, window: int, now: floa (spent inside the window). One write, so two identical requests cannot both acquire; a crashed owner's lease simply expires. Spent rows are pruned in `complete_signature`, so the table is the size of the window. + A replay of a handled request is answered from a read, no write lock. """ + cursor = await self.read.execute( + "SELECT 1 FROM spent_signatures WHERE token = ? AND seen_at >= ? LIMIT 1", (token, now - window) + ) + if await cursor.fetchone(): + return "completed" async with self._write(): await self.db.execute("DELETE FROM pending_signatures WHERE expires_at <= ?", (now,)) cursor = await self.db.execute( diff --git a/hookrelay/tests/test_signature_replay.py b/hookrelay/tests/test_signature_replay.py index c6ee992..2183df4 100644 --- a/hookrelay/tests/test_signature_replay.py +++ b/hookrelay/tests/test_signature_replay.py @@ -20,7 +20,6 @@ import json import httpx -import pytest import hookrelay.app as app_module from hookrelay.app import create_app @@ -220,27 +219,27 @@ async def test_separate_stores_serialize_signature_acquisition(tmp_path): await second.close() -async def test_failed_lease_renewal_cancels_processing_and_releases_lease(settings, cfg, monkeypatch): - entered = asyncio.Event() - real_handle = app_module.handle_hook - real_sleep = asyncio.sleep - calls = 0 +async def test_a_failed_renewal_lets_the_request_finish(settings, cfg, monkeypatch): + """Cancelling on a failed renewal left the sender a dropped connection and + the pipeline half-run. The lease is still the request's, so it completes, + and the next identical copy is the replay it is.""" + entered, failed = asyncio.Event(), asyncio.Event() + real_handle, real_sleep = app_module.handle_hook, asyncio.sleep async def fast_heartbeat(delay): if delay == 40: await entered.wait() - else: - await real_sleep(delay) + await real_sleep(0.01) + return + await real_sleep(delay) async def handle(*args, **kwargs): - nonlocal calls - calls += 1 - if calls == 1: - entered.set() - await asyncio.Event().wait() + entered.set() + await failed.wait() return await real_handle(*args, **kwargs) async def broken_renew(*args, **kwargs): + failed.set() raise OSError("database unavailable") monkeypatch.setattr(app_module.asyncio, "sleep", fast_heartbeat) @@ -250,12 +249,55 @@ async def broken_renew(*args, **kwargs): app = create_app(settings=settings, cfg=cfg) async with _serve(app) as client: monkeypatch.setattr(app.state.store, "renew_signature", broken_renew) - with pytest.raises(asyncio.CancelledError): - await asyncio.wait_for(client.post("/hook/grafana", content=body, headers=headers), 2) + first = await asyncio.wait_for(client.post("/hook/grafana", content=body, headers=headers), 5) monkeypatch.setattr(app_module.asyncio, "sleep", real_sleep) + again = await client.post("/hook/grafana", content=body, headers=headers) + assert first.status_code == 200 and first.json()["outcome"] == "routed" + assert again.json()["skip_code"] == "replayed_signature" + + +async def test_a_lease_lost_before_completion_answers_503(settings, cfg, monkeypatch): + async def lost(*args, **kwargs): + return False + + body = json.dumps(PAYLOAD).encode() + headers = {"X-Hook-Signature": _sig("s3cret", body)} + app = create_app(settings=settings, cfg=cfg) + async with _serve(app) as client: + monkeypatch.setattr(app.state.store, "complete_signature", lost) response = await client.post("/hook/grafana", content=body, headers=headers) - assert response.status_code == 200 and response.json()["outcome"] == "routed" - assert calls == 2 + assert response.status_code == 503 and "lease lost" in response.json()["detail"] + + +async def test_a_replay_of_a_handled_request_takes_no_write_lock(settings, cfg, monkeypatch): + """A bulk replay of one capture used to cost a write per copy.""" + body = json.dumps(PAYLOAD).encode() + headers = {"X-Hook-Signature": _sig("s3cret", body)} + app = create_app(settings=settings, cfg=cfg) + async with _serve(app) as client: + assert (await client.post("/hook/grafana", content=body, headers=headers)).json()["outcome"] == "routed" + store, writes = app.state.store, [] + real_write = store._write + + def counting_write(): + writes.append(1) + return real_write() + + monkeypatch.setattr(store, "_write", counting_write) + for _ in range(5): + again = await client.post("/hook/grafana", content=body, headers=headers) + assert again.json()["skip_code"] == "replayed_signature" + assert writes == [], "a replay of a handled request wrote to the ledger" + + +def test_the_lease_length_is_a_setting_with_a_floor(monkeypatch): + from hookrelay.settings import Settings + + assert Settings.load().signature_lease_seconds == 120 + monkeypatch.setenv("HOOKRELAY_SIGNATURE_LEASE_SECONDS", "45") + assert Settings.load().signature_lease_seconds == 45 + monkeypatch.setenv("HOOKRELAY_SIGNATURE_LEASE_SECONDS", "3") + assert Settings.load().signature_lease_seconds == 10 def test_nonfinite_signed_timestamps_are_refused(): diff --git a/scripts/assert_weight.py b/scripts/assert_weight.py index 9ca3d78..b12a61b 100755 --- a/scripts/assert_weight.py +++ b/scripts/assert_weight.py @@ -332,7 +332,14 @@ # +~49 code. The doctrine question: this is the pipe's own noise valve, not # a judgement about an alert; the content it reads is the title it already # extracted for routing. - "hookrelay": (6650, Path("hookrelay/README.md")), + # 6650 -> 6700 on 2026-10-27, for the signature lease's follow-ups (PR #24 + # landed at exactly 6650 by trading dead methods for restored reasoning): a + # replay of a handled request is answered from a read instead of a write per + # copy, a failed renewal no longer cancels the request mid-pipeline, and the + # lease length is a setting. Split: +~9 source over the line, the rest is + # headroom. The doctrine question: the pipe's own door and ledger, not a + # judgement about an alert. + "hookrelay": (6700, Path("hookrelay/README.md")), # 2900 -> 3000 on 2026-08-21, for the judge's second axis (`wake_someone`). # Raised rather than trimmed because the thing that pushed it over is the one # measurement that says whether this service earns its model calls at all: diff --git a/scripts/reproduce_claims.py b/scripts/reproduce_claims.py index 8e35f5b..38c9495 100755 --- a/scripts/reproduce_claims.py +++ b/scripts/reproduce_claims.py @@ -70,7 +70,7 @@ def _pytest_claim(test_path: str, venv_service: str) -> tuple[bool | None, str]: # (claim, how to check it, artefact for the reader) REPRODUCIBLE: tuple[tuple[str, str, object], ...] = ( ( - "hookrelay stays at or under 6,650 source lines", + "hookrelay stays at or under 6,700 source lines", "hookrelay/README.md", lambda: _run([sys.executable, "scripts/assert_weight.py"]), ),