[PR #4043] fix(setup): write the recommended tool call timeout into dsh registrations #6012

Open
opened 2026-10-31 01:11:10 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4043
Author: @kittimzhe
Created: 10/15/2026
Status: 🔄 Open

Base: main ← Head: fix/dsh-tool-call-timeout


📝 Commits (3)

  • a41045e fix(setup): write the recommended tool call timeout into dsh registrations
  • 4677f6e fix(setup): keep dsh expressions through budget-gated setup and doctor
  • f22153e fix(setup): derive the dsh deadline with a declared budget read set

📊 Changes

9 files changed (+426 additions, -52 deletions)

View changed files

📝 docs/client-configuration.md (+12 -0)
📝 src/application/ClientRegistration.ts (+6 -0)
📝 src/application/ClientRegistrationEnvironment.ts (+19 -5)
📝 src/application/ClientRegistrationStatus.ts (+21 -8)
📝 src/application/Doctor.ts (+38 -16)
📝 src/application/DshCordisPatch.ts (+25 -0)
📝 src/application/SetupClientConfiguration.ts (+12 -2)
📝 src/doctorRuntime.ts (+27 -21)
📝 tests/boundary/filesystem/dshClientConfiguration.test.ts (+266 -0)

📄 Description

Summary

rea setup --client dsh wrote registrations without toolCallTimeoutMs. DeepSeek Harness's MCP client applies a 60000 ms default for that field, while REA's known native provider allowance (the default 330-second Ghidra startup) alone exceeds it, so a tool call could be cut off during provider startup before analysis began. Setup now derives the registration timeout from the same provider allowance plus 120 seconds of operation headroom used for the Kimi and Codex registrations, and doctor reports pre-existing registrations for repair.

Problem and expected behavior

Smallest trigger: run rea setup --client dsh and inspect ~/.dsh/cordis.patch.yml — the REA entry carries only serverName, transport, command, args (and an optional env map).

Violated invariant: registration refresh derives a recommended tool deadline for codex (tool_timeout_sec) and kimi (toolTimeoutMs) from the validated effective environment's timeoutInputs, but the dsh branch of clientConfigurationDesired did not consult the environment details at all, and DshCordisDocument.desired() had no timeout to write.

Expected: a dsh registration carries toolCallTimeoutMs covering the selected provider allowance plus operation headroom; a positive caller-selected value is preserved.

Change and scope

  • DshCordisDocument.desired() takes the recommended timeout and preserves a positive caller-selected toolCallTimeoutMs; any other value (malformed literal, unevaluated !!js expression) is replaced with the recommended literal, matching how registration-status expressions are repaired elsewhere. The Cordis serialization carries the field.
  • The dsh branch of clientConfigurationDesired uses clientRegistrationEnvironmentDetailsWithBudget with the provider request budget resolver. ProviderRequestBudgetResolver declares an optional budgetSettings read set owned by the resolver implementation, so the fail-closed gate applies only to settings the derivation may actually read plus the timeout inputs: unrelated Cordis expressions (for example GHIDRA_INSTALL_DIR: !!js process.env.GHIDRA_INSTALL_DIR) still refresh, an all-literal environment derives the upstream request budget into the written deadline, and an expression in a budget-read setting fails closed instead of being treated as absent. The gate's behavior for codex and kimi is unchanged.
  • parseClientRegistration's dsh pick keeps toolCallTimeoutMs as a tolerant optional: an unevaluated Cordis expression or malformed literal parses to undefined (not an invalid registration), so alignment reports it stale and setup repairs it with the recommended literal.
  • Doctor: dsh joins the registration-environment computation; a registration without a usable toolCallTimeoutMs is reported stale; timeoutAdvisory covers dsh (ms unit, sharing the Kimi formatting and the batch-scope guidance).
  • Rebased onto current main: the earlier structural fix (unresolved-expression context through parseEnvironment) landed upstream independently and is no longer part of this diff; the remaining change is the timeout write itself plus doctor coverage.
  • Contribution scope: ordinary fix within existing registration boundaries — no maintainer scope decision required ("not applicable").

Contract and boundary impact

  • Semantic owner and earliest changed stage: setup's dsh registration computation (DshCordisDocument.desired) and the doctor registration status (ClientRegistrationStatus).
  • CLI and MCP/tool-catalog contract: the written entry gains toolCallTimeoutMs; rea doctor --client dsh gains the stale state and the advisory for this field. No tool-catalog change.
  • Provider, bridge, target-format, or platform compatibility: none — registration shape only.
  • Evidence, artifact, provenance contract: none.
  • Generated metadata: none — no catalog or skill outputs touched.

Expected effects:

  • Read existing files or captured data.
  • Launch or attach to a process.
  • Inject instrumentation or run scripts/code.
  • Change process memory, registers, files, or configuration. (writes the caller's own client registration file during setup; no analysis target interaction)
  • Contact network services.
  • Recover protected code or work around protections.
  • Documentation or usability only; no target interaction.
  • Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

  • Local RED (executed before the original change, still true on current main): a dsh registration written by rea setup --client dsh carries no toolCallTimeoutMs (grep of src/ on current main confirms the field appears nowhere).
  • Tests: the dsh configuration suite covers the timeout write, preservation of positive caller values, replacement of unevaluated expressions and malformed literals, doctor staleness, the advisory wording, budget-resolver-supplied setup and doctor runs (an unrelated environment expression keeps refreshing, the declared upstream budget lands in the written deadline, and a budget-read setting expression fails closed), within tests/boundary/filesystem/dshClientConfiguration.test.ts; tests/boundary/filesystem/clientRegistrationStatus.test.ts covers the status flow.
  • Remaining proof gaps: an end-to-end dsh session with a slow provider startup was not run; the 60000 ms client default is dsh client behavior, not enforced by REA.

For evidence-bearing changes:

  • Observed, derived, and inferred claims remain distinguishable.
  • Artifact identity, source provenance, and failed attempts remain preserved.
  • Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

Executed on the rebased branch (base 25f0f9934, Node 24.22.1, macOS 14 arm64):

  • npm ci — locked dependencies.
  • npm run build:cached — compile passes.
  • npm run test:focused -- tests/boundary/filesystem/dshClientConfiguration.test.ts — 109 passed.
  • npm run test:focused -- tests/boundary/filesystem/clientRegistrationStatus.test.ts — 39 passed.
  • npm run check:fast — typecheck + lint pass.

Compatibility, safety, and release

  • Breaking changes or migration steps: none. Existing dsh registrations gain a field on the next setup refresh; a positive caller-selected toolCallTimeoutMs is preserved unchanged.
  • Real Hopper/Ghidra, browser, or OS coverage: not applicable — no provider is launched.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: none.

Author readiness

Human owner: @kittimzhe

  • I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
  • I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
  • I declare this PR ready for initial maintainer review.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked: none — the defect is observable directly from rea setup --client dsh output.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/4043 **Author:** [@kittimzhe](https://github.com/kittimzhe) **Created:** 10/15/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/dsh-tool-call-timeout` --- ### 📝 Commits (3) - [`a41045e`](https://github.com/morluto/rea/commit/a41045eb8fb908a0dc73871d54e03ccd4ade2752) fix(setup): write the recommended tool call timeout into dsh registrations - [`4677f6e`](https://github.com/morluto/rea/commit/4677f6eca5955583be62459a7878840895bb66c4) fix(setup): keep dsh expressions through budget-gated setup and doctor - [`f22153e`](https://github.com/morluto/rea/commit/f22153e7a4a0ac77f9edd791ea2cc6b08e15c8c0) fix(setup): derive the dsh deadline with a declared budget read set ### 📊 Changes **9 files changed** (+426 additions, -52 deletions) <details> <summary>View changed files</summary> 📝 `docs/client-configuration.md` (+12 -0) 📝 `src/application/ClientRegistration.ts` (+6 -0) 📝 `src/application/ClientRegistrationEnvironment.ts` (+19 -5) 📝 `src/application/ClientRegistrationStatus.ts` (+21 -8) 📝 `src/application/Doctor.ts` (+38 -16) 📝 `src/application/DshCordisPatch.ts` (+25 -0) 📝 `src/application/SetupClientConfiguration.ts` (+12 -2) 📝 `src/doctorRuntime.ts` (+27 -21) 📝 `tests/boundary/filesystem/dshClientConfiguration.test.ts` (+266 -0) </details> ### 📄 Description ## Summary `rea setup --client dsh` wrote registrations without `toolCallTimeoutMs`. DeepSeek Harness's MCP client applies a **60000 ms** default for that field, while REA's known native provider allowance (the default 330-second Ghidra startup) alone exceeds it, so a tool call could be cut off during provider startup before analysis began. Setup now derives the registration timeout from the same provider allowance plus 120 seconds of operation headroom used for the Kimi and Codex registrations, and doctor reports pre-existing registrations for repair. ## Problem and expected behavior Smallest trigger: run `rea setup --client dsh` and inspect `~/.dsh/cordis.patch.yml` — the REA entry carries only `serverName`, `transport`, `command`, `args` (and an optional `env` map). Violated invariant: registration refresh derives a recommended tool deadline for codex (`tool_timeout_sec`) and kimi (`toolTimeoutMs`) from the validated effective environment's `timeoutInputs`, but the dsh branch of `clientConfigurationDesired` did not consult the environment details at all, and `DshCordisDocument.desired()` had no timeout to write. Expected: a dsh registration carries `toolCallTimeoutMs` covering the selected provider allowance plus operation headroom; a positive caller-selected value is preserved. ## Change and scope - `DshCordisDocument.desired()` takes the recommended timeout and preserves a positive caller-selected `toolCallTimeoutMs`; any other value (malformed literal, unevaluated `!!js` expression) is replaced with the recommended literal, matching how registration-status expressions are repaired elsewhere. The Cordis serialization carries the field. - The dsh branch of `clientConfigurationDesired` uses `clientRegistrationEnvironmentDetailsWithBudget` with the provider request budget resolver. `ProviderRequestBudgetResolver` declares an optional `budgetSettings` read set owned by the resolver implementation, so the fail-closed gate applies only to settings the derivation may actually read plus the timeout inputs: unrelated Cordis expressions (for example `GHIDRA_INSTALL_DIR: !!js process.env.GHIDRA_INSTALL_DIR`) still refresh, an all-literal environment derives the upstream request budget into the written deadline, and an expression in a budget-read setting fails closed instead of being treated as absent. The gate's behavior for codex and kimi is unchanged. - `parseClientRegistration`'s dsh pick keeps `toolCallTimeoutMs` as a tolerant optional: an unevaluated Cordis expression or malformed literal parses to `undefined` (not an invalid registration), so alignment reports it stale and setup repairs it with the recommended literal. - Doctor: dsh joins the registration-environment computation; a registration without a usable `toolCallTimeoutMs` is reported stale; `timeoutAdvisory` covers dsh (ms unit, sharing the Kimi formatting and the batch-scope guidance). - Rebased onto current main: the earlier structural fix (unresolved-expression context through `parseEnvironment`) landed upstream independently and is no longer part of this diff; the remaining change is the timeout write itself plus doctor coverage. - Contribution scope: ordinary fix within existing registration boundaries — no maintainer scope decision required ("not applicable"). ## Contract and boundary impact - Semantic owner and earliest changed stage: setup's dsh registration computation (`DshCordisDocument.desired`) and the doctor registration status (`ClientRegistrationStatus`). - CLI and MCP/tool-catalog contract: the written entry gains `toolCallTimeoutMs`; `rea doctor --client dsh` gains the stale state and the advisory for this field. No tool-catalog change. - Provider, bridge, target-format, or platform compatibility: none — registration shape only. - Evidence, artifact, provenance contract: none. - Generated metadata: none — no catalog or skill outputs touched. Expected effects: - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [x] Change process memory, registers, files, or configuration. *(writes the caller's own client registration file during setup; no analysis target interaction)* - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Local RED (executed before the original change, still true on current main): a dsh registration written by `rea setup --client dsh` carries no `toolCallTimeoutMs` (grep of `src/` on current main confirms the field appears nowhere). - Tests: the dsh configuration suite covers the timeout write, preservation of positive caller values, replacement of unevaluated expressions and malformed literals, doctor staleness, the advisory wording, budget-resolver-supplied setup and doctor runs (an unrelated environment expression keeps refreshing, the declared upstream budget lands in the written deadline, and a budget-read setting expression fails closed), within `tests/boundary/filesystem/dshClientConfiguration.test.ts`; `tests/boundary/filesystem/clientRegistrationStatus.test.ts` covers the status flow. - Remaining proof gaps: an end-to-end dsh session with a slow provider startup was not run; the 60000 ms client default is dsh client behavior, not enforced by REA. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed Executed on the rebased branch (base `25f0f9934`, Node 24.22.1, macOS 14 arm64): - `npm ci` — locked dependencies. - `npm run build:cached` — compile passes. - `npm run test:focused -- tests/boundary/filesystem/dshClientConfiguration.test.ts` — 109 passed. - `npm run test:focused -- tests/boundary/filesystem/clientRegistrationStatus.test.ts` — 39 passed. - `npm run check:fast` — typecheck + lint pass. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Existing dsh registrations gain a field on the next setup refresh; a positive caller-selected `toolCallTimeoutMs` is preserved unchanged. - Real Hopper/Ghidra, browser, or OS coverage: not applicable — no provider is launched. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none. ## Author readiness Human owner: @kittimzhe - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [ ] This PR addresses a concrete problem or an agreed enhancement. - [ ] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked: none — the defect is observable directly from `rea setup --client dsh` output. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
morluto/rea#6012
No description provided.