refactor(herdr): type the herdr RPC seam and route callers through wrappers

[devswha]

Problem

server/herdr/client.ts exported an untyped herdrRpc(method: string, params: Record<string, unknown>). Eleven server files called it directly for four methods (pane.process_info x10, agent.get x6, pane.send_text x2, pane.send_keys x1), each with its own inline result type. Nothing stopped { target } reaching pane.process_info, which herdr then silently answers for the focused pane (the AGENTS.md rule was prose only).

Change

• server/herdr/methods.ts: HerdrMethods, a { [method]: { params; result } } map for those four methods. Params and result bodies come from the generated schema types; PaneProcessInfoParams, AgentTarget and PaneProcessInfo are now generator roots (scripts/generate-protocol-types.ts, regenerated shared/herdr-api.generated.ts). The map itself is hand-written because herdr's schema links a method to its params (request.oneOf) but not to its answer: answers are one ResponseResult.oneOf told apart only by a type tag (pane_process_info, agent_info, ok).
• herdrRpc<M extends keyof HerdrMethods>(method, params: HerdrMethods[M]["params"], …): Promise<HerdrMethods[M]["result"]> is the first overload. An untyped fallback overload stays for methods the map does not list yet. It refuses a typed method name (UntypedHerdrMethod<M> is never for one), so herdrRpc("pane.process_info", { target }) is a compile error. server/herdr/seam.test.ts pins that with @ts-expect-error and checks the wire params of the new wrappers against a stand-in socket.
• New wrappers: paneProcessInfo(paneId) returns one shared ProcessInfo (= generated PaneProcessInfo: shell_pid, foreground_processes with pid/name/argv0/argv) and agentGet(paneId) returns AgentInfo. The existing paneSendText / paneSendKeys now go through the typed overload. The wrappers call the exported herdrRpc binding, so tests that spyOn(herdr, "herdrRpc") keep working.
• Migrated (no open PR touches them): server/pi.ts, server/omp.ts, server/grok-store.ts, server/agy-store.ts, server/codex.ts (x3). Behaviour is unchanged: the same params go on the wire, with the same default timeout and the same .catch / try at each call site. codex.ts keeps its old argv?: string[] reading through a commented cast, because the schema allows argv: null and that path has never handled null. Changing that would change behaviour, so it is left for a separate fix.
• Guard: server/conventions.test.ts ("herdr socket") now fails when server code outside server/herdr/ uses herdrRpc. The message names the fix ("add a typed wrapper in server/herdr/client.ts; for pane status use the server/collector.ts listeners") and the allowlist HERDR_RPC_ALLOWED. A stale allowlist entry also fails. The events.subscribe / subscribeEvents rules from test: enforce AGENTS.md conventions with repo-wide guard tests #949 were already there; their message now also says where events.subscribe lives.
• AGENTS.md: the pane.process_info line points at paneProcessInfo / HerdrMethods and names the enforcing test. Only that one line changes; feat(shortcuts): record and preview custom bindings #812, feat(i18n): add Russian as a UI language #888 and fix(chat): type a message sent during a turn into the agent at once #894 also edit AGENTS.md.

Allowlist (HERDR_RPC_ALLOWED, migrated in PR B)

File	Open PRs touching it
server/conversation.ts	#869 #912 #923
server/gjc-runtime.ts	#912
server/hermes.ts	#910
server/omo.ts	#923
server/shell-agent.ts	#885 #923

Built from gh pr list --state open --limit 100 --json number,files on 2026-10-14.

Known limits (closed in PR B)

• The export is still named herdrRpc. Renaming it to herdrRpcUnsafe would touch about 33 test files and 30 scripts/ harnesses, several of them in open PRs (api.contract, gjc.contract, omo.contract, ui-regression, pending-input…). Tests and scripts/ QA harnesses are outside this guard's scope: they drive herdr (pane.report_agent, workspace.create, …) and are not part of the bridge seam.
• A call with an explicit result type, herdrRpc<Inline>("pane.process_info", …), still reaches the untyped fallback. That is the form the allowlisted files use, and they may not change here. In server code the source guard closes this gap. The fallback goes away once the allowlist is empty.

Validation

• bun run check fast: green. A first run had one failure in server/portal.test.ts ("does not treat an arbitrary interpreter as a Portal script": the process argv read raced and returned undefined). This change does not touch that file, it passed alone, and the rerun was fully green.
• bun run test:integration: 455 pass, 0 fail, 14 skip in 38 files (includes the omp, codex, codex-binding, pi, gjc, omo and hermes contract tests).
• I checked by hand that the guard fails on a new herdr.herdrRpc use in server/pi.ts.

Follow-up: PR B

Once #869, #885, #910, #912 and #923 land: migrate the five allowlisted files to paneProcessInfo / agentGet / paneSendText / paneSendKeys, empty HERDR_RPC_ALLOWED, drop the untyped fallback overload (or rename it herdrRpcUnsafe for tests and scripts), and consider handling argv: null in codex.ts.

Review

Fixed in 290d5f1:

• Union of typed methods (medium, part of finding 1). The typed overload takes SingleHerdrMethod<M>, so herdrRpc(cond ? "pane.process_info" as const : "agent.get" as const, { target }) no longer compiles. seam.test.ts pins this with a @ts-expect-error line. Checked with tsc: the inferred herdrRpc("pane.process_info", { pane_id }) still returns the typed result.
• Allowlist missed fix(gjc): derive status from the native parent turn #928 and feat(hermes): add safe Chat interaction #859 (medium). conventions.test.ts adds HERDR_RPC_PENDING with server/gjc-status.ts (fix(gjc): derive status from the native parent turn #928) and server/index.ts (feat(hermes): add safe Chat interaction #859). Those PRs add a pane.process_info call, but neither file calls herdrRpc on main yet, so the "no longer calls" check skips them. It would be better for both PRs to switch to paneProcessInfo(paneId), which is a one-line change in each. After that, these entries come out.
• Docs claimed more than the PR delivers (low). AGENTS.md now says the guard enforces the rule, not the compiler, and that the allowlisted and pending files still call herdrRpc. The changelog fragment now says "no longer compiles without an explicit result type". "PR B" is replaced with "a follow-up PR that moves these files to the wrappers once their open PRs land". The fallback docblock in client.ts and the UntypedHerdrMethod comment in methods.ts list the holes.

Deferred to the follow-up PR that drains HERDR_RPC_ALLOWED:

• Explicit result type and widened string (rest of finding 1). herdrRpc<T>("pane.process_info", { target }) and herdrRpc(m as string, { target }) still compile. With an explicit T, the fallback's M stays at string, and that cannot be closed while the fallback shares the name. Closing it means splitting the fallback out as herdrRpcUntyped (or herdrRpcUnsafe) and moving about 400 test and script call sites onto it. It also means converting the herdrRpc<T>("pane.process_info" | "agent.get", …) calls in the five allowlisted files, which conflicts with their open PRs. seam.test.ts keeps both forms as compiling lines with a comment, so the follow-up can turn them into @ts-expect-error.
• Error message. Two overloads are still present, so tsc reports a bad inferred call such as { target } to pane.process_info as not assignable to parameter of type 'never' instead of naming the wrong param. That message only gets better with the split.
• Stale pending entries. Nothing removes a HERDR_RPC_PENDING entry on its own. If fix(gjc): derive status from the native parent turn #928 or feat(hermes): add safe Chat interaction #859 switches to paneProcessInfo or closes, its entry stays until someone takes it out by hand.
• Tell the open PRs. fix(gjc): bind chat sessions through the SDK broker #912, fix(omo): bind custom runtimes by native session ownership #923, fix(gjc): derive status from the native parent turn #928, feat(hermes): add safe Chat interaction #859 and feat(hermes): discover native skill commands and render skill invocations #910 call herdrRpc<T>("pane.process_info", …). They should hear about the switch to paneProcessInfo before the split lands. I have not commented on those PRs.

Checks: bun run check fast passed (typecheck, build, 3452 unit tests). bun run test:integration passed (455 pass, 0 fail).

🤖 Generated with Claude Code

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: devswha/herdr-web-ui/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 457c005d-08f5-4aac-a74a-1ca0a770b773

📥 Commits

Reviewing files that changed from the base of the PR and between a338f8b and 9474db5.

⛔ Files ignored due to path filters (1)

• shared/herdr-api.generated.ts is excluded by !**/*.generated.*, !shared/herdr-api.generated.ts

📒 Files selected for processing (14)

• AGENTS.md
• changelog.d/herdr-rpc-seam.md
• scripts/generate-protocol-types.ts
• server/agy-store.ts
• server/codex.ts
• server/conventions.test.ts
• server/conversation.test.ts
• server/grok-store.ts
• server/herdr/client.ts
• server/herdr/methods.ts
• server/herdr/seam.test.ts
• server/omp.test.ts
• server/omp.ts
• server/pi.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review.

📝 Summary

Summary by CodeRabbit

• Refactor
  • Pane process and agent information are now retrieved through consistent, typed requests. Pane process lookups use the pane ID, helping ensure information corresponds to the requested pane.
  • Existing process and session checks remain unchanged; no user-facing behavior changes are noted.
• Tests
  • Added checks for request parameters and returned pane and agent information, plus safeguards against untyped requests in most server code.

Herdr RPC methods now have typed parameter and response contracts, with client wrappers for pane process information and agent details. Server consumers use those wrappers, and a convention test limits direct herdrRpc calls.

Changes: Herdr RPC typing and migration

Layer / File(s)	Summary
Define Herdr method contracts scripts/generate-protocol-types.ts, server/herdr/methods.ts	The protocol type generator includes the required request and response types. The method registry defines parameter and response types for four RPC methods.
Add typed client wrappers server/herdr/client.ts, server/herdr/seam.test.ts	herdrRpc adds a method-specific typed overload and retains an untyped fallback for other methods. paneProcessInfo and agentGet unwrap their respective responses. Tests check parameter typing and wrapper requests and results.
Migrate server consumers and enforce the seam server/agy-store.ts, server/codex.ts, server/grok-store.ts, server/omp.ts, server/pi.ts, server/conventions.test.ts, server/conversation.test.ts, server/omp.test.ts, AGENTS.md, changelog.d/herdr-rpc-seam.md	Server consumers use the new wrappers for pane process information or agent details. The convention test restricts direct herdrRpc calls, and supporting guidance, changelog text, and test mock types are updated.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to 9474d

No actionable issue remains from the reviewed changes; the PR is mergeable after normal checks.

Pre-merge checks · 4 · 1

Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 12 files. (2 skipped:…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Title check		The title clearly and concisely describes the main change: typing the Herdr RPC seam and routing callers through wrappers.
Description check		The description includes the required change and validation information. It explains the problem, implementation, known limits, follow-up work, and reports relevant checks and results.

Full details: Docstring Coverage

Explanation

Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 12 files. (2 skipped: 2 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}