feat(plugins): send a cancel notification to the plugin on call timeout

[debba]

Stacked on #833 (base is feat/configurable-plugin-call-timeout). Retarget to main once #833 is merged.

Host side of #832. When a plugin call timed out, the host only dropped its own pending entry, so the plugin kept working and a Postgres statement kept running on the server (a DELETE still took effect after the user saw an error). The plugin side already ships in postgresql-plugin 1.0.0-rc.6 (TabularisDB/tabularis-postgresql-plugin#127), using the envelope agreed in the issue.

What changed

• plugins/rpc.rs: new JsonRpcNotification type and cancel_notification_line(id), which builds the line written to the plugin:

  {"jsonrpc":"2.0","method":"cancel","params":{"id":<request id>}}

  There is no top-level id, so it is a real JSON-RPC notification and the plugin must not reply.
• plugins/driver.rs: the management task already handled PluginCommand::Cancel(id) by removing the pending entry. It now also writes the cancel notification to stdin, but only if the entry was still pending: a response that raced the timeout does not trigger a cancel. With the timeout disabled (0, from feat(plugins): configurable plugin call timeout with per-plugin override #833) the timeout branch is never reached, so nothing is sent.
• plugins/PLUGIN_GUIDE.md: new "Cancel Notification (Optional)" section (do not reply, ignore unknown ids, keep reading stdin while a request runs).

Plugins that do not handle cancel keep working: if they answer the notification anyway (e.g. with "method not found"), that line matches no pending request and is dropped, at worst with one Failed to parse plugin response log line.

Verification

• rpc_tests.rs: the line is a single newline-terminated notification, has no top-level id, and keeps u64::MAX exact.
• cancel_tests.rs (unix, real child process via /bin/sh recording stdin): a timed-out call is followed by a cancel carrying the same id; a call with no timeout never sends one.
• cargo test --lib plugins:: passes (147 tests), no clippy warnings in the touched files.
• Manual end-to-end against a local Postgres 16 with a throwaway test driving the real plugin binary, SELECT pg_sleep(60) and a 3s timeout:
  • rc.6: statement active during the call, host error at 3.0s, no backend left 500ms later.
  • older installed plugin (control): statement still active after the timeout.

[kilo-code-bot]

SUGGESTION: Fragile parse of a log file that is still being written

wait_for_lines polls stdin.log while the spawned cat is appending to it, so a poll can observe a partially written line (the last, not-yet-terminated chunk). serde_json::from_str(line).unwrap() then panics and the test fails intermittently instead of just seeing fewer lines. Filtering unparsable lines makes the helper tolerant of the concurrent writer:

Suggested change

	.map(|line| serde_json::from_str(line).unwrap())
	.filter_map(|line| serde_json::from_str(line).ok())

---

Reply with @kilocode-bot fix it to have Kilo Code address this issue.

[kilo-code-bot]

SUGGESTION: Document that the cancel is only sent when the request is still pending

The host only writes the notification while the entry is still pending (if pending_requests.remove(&id).is_some() in driver.rs), so a response that raced the timeout produces no cancel. The wording "Right after that it writes" reads as unconditional; plugin authors could end up relying on always receiving a cancel. Worth stating that no cancel arrives for a request that already completed.

---

Reply with @kilocode-bot fix it to have Kilo Code address this issue.

[kilo-code-bot]

Code Review Summary

Status: 2 Issues Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	0
WARNING	0
SUGGESTION	2

Issue Details (click to expand)

SUGGESTION

File	Line	Issue
src-tauri/src/plugins/cancel_tests.rs	24	wait_for_lines unwraps a JSON parse of a log file still being appended by the child cat, so a partially written line panics the test intermittently
plugins/PLUGIN_GUIDE.md	678	Docs say the cancel is written unconditionally, but it is only sent when the request is still pending

Files Reviewed (5 files)

• plugins/PLUGIN_GUIDE.md - 1 issue
• src-tauri/src/plugins/cancel_tests.rs - 1 issue
• src-tauri/src/plugins/driver.rs - 0 issues
• src-tauri/src/plugins/rpc.rs - 0 issues
• src-tauri/src/plugins/rpc_tests.rs - 0 issues

Fix these issues in Kilo Cloud

---

_{Reviewed by free · Input: 0 · Output: 0 · Cached: 0}

[aesslinger]

Reviewed the driver.rs change (+ rpc.rs helper, cancel_tests.rs, PLUGIN_GUIDE.md) — LGTM. The envelope matches our rc.6 contract exactly, and the race guard is the right design.

driver.rs — correct

The timeout-fire branch now does two things:

if pending_requests.remove(&id).is_some() {
    let line = cancel_notification_line(id);
    if let Err(e) = stdin.write_all(line.as_bytes()).await { ... log::warn ... }
}

The key correctness detail is the if ... .is_some() guard: the cancel notification is sent only if the request was still pending. This handles the race where the plugin's response arrived between the timeout firing and the management task processing Cancel — remove returns None and no notification goes out. You don't cancel an id whose response you already got, and a duplicate Cancel (two timeouts for the same id) sends at most one notification.

The write goes to the same stdin the call path uses, so it's serialized correctly with outgoing requests (no interleaving). A write failure is logged and swallowed — correct, since the host already reported the timeout error to the user; a failed cancel isn't actionable.

Contract match with our rc.6 — exact

• No top-level id: JsonRpcNotification has no id field, so serde omits it. Our handle_line detects notification-ness by the absence of the id field → skips the stdout write so no stray response line corrupts the protocol stream. ✓ (Your test lines[1] asserts the notification has no id key.)
• params.id as u64: json!({ "id": id }) with id: u64 → our handler reads params.get("id").and_then(Value::as_u64). ✓
• Unknown id is a no-op: our cancel::cancel(u64) returns silently for an unregistered id; the guide documents this. ✓
• Fire-and-forget: no response channel, log-on-failure. ✓

rpc.rs helper + tests — solid

cancel_notification_line builds the notification via a typed JsonRpcNotification struct (not string interpolation) — type-safe, no escaping concerns. The cancel_tests.rs silent-recording plugin (fd 3 trick to keep stdout open) verifies the request + cancel lines both land on stdin, and the separate test confirms None timeout never sends a cancel. Good coverage of the two branches.

One note on the guide's "Keep reading stdin" point

The guide says "Keep reading stdin while a request runs. A plugin that processes requests strictly one at a time only sees the cancel after the long call has finished." — worth noting that the PostgreSQL plugin's worker-pool architecture (4 workers, main.rs) already satisfies this: a long query on one worker doesn't block the reader from dispatching the cancel notification to another worker, so the cancel arrives and is acted on while the sleep is still running. Your end-to-end test (pg_sleep(60) + 3s timeout, backend gone with rc.6) already confirmed this works, but it's worth knowing why it works — a strictly-serial plugin would need to read stdin concurrently with the in-flight query, and ours does.

No blockers from our side — once #842 and its base #833 land, the cancel feature lights up for all postgresql-plugin 1.0.0-rc.6+ users with no further plugin-side work.

[debba]

Merging it