diff --git a/.changeset/remove-dead-limbo-timeout.md b/.changeset/remove-dead-limbo-timeout.md new file mode 100644 index 00000000..14b1e5ab --- /dev/null +++ b/.changeset/remove-dead-limbo-timeout.md @@ -0,0 +1,38 @@ +--- +"@paddock/server": minor +"@paddock/web": patch +--- + +Remove the `recovery.limboTimeoutMs` lever, which never did anything + +`limboTimeoutMs` / `PADDOCK_RECOVERY_LIMBO_MS` was parsed (`config.ts`), +defaulted (`recovery-config.ts`), sanitised, resolved per-project, carried into +the web types — and **read by nothing**. There were zero consumers in +`packages/server/src`. It has been inert since it was introduced with #301. + +The docs were honest about it (*"Backstop timer ships in a follow-up"*), but the +Settings UI was not: `instance-config.ts` listed it as an `editable: true` field +alongside levers that work, with no indication it was a no-op. Anyone who set it +— including this project's own dev box, which exports +`PADDOCK_RECOVERY_LIMBO_MS=60000` — got silence and no way to tell. + +Its original purpose was a backstop for a chat wedged "running" with no way out. +**#528 removed that need**: Stop now works during a chat's background phase, so a +wedged session is escapable from the UI rather than needing a timer to notice it. +Rather than implement a timer nothing is waiting for, the lever goes. + +Removed end to end: the config key and env var, the Settings field, the +`RecoveryConfig` / `RecoveryOverride` member, and the docs rows describing it. + +**Nothing breaks for existing installs.** `sanitizeRecoveryOverride` is an +allowlist, so a `limboTimeoutMs:` left in a `project.yaml` or +`paddock.config.yaml` is silently ignored — exactly the effect it has today. The +env var simply stops being read. No `schemaVersion` bump: the bump rule is +"remove a *load-bearing* key", and this one carried no load, which is the point. + +Worth knowing if you provision Paddock with config management: any +`PADDOCK_RECOVERY_LIMBO_MS` in your environment is now dead weight and can be +dropped. + +The remaining recovery levers — `surfaceKilledTask`, `autoReDrive`, +`debounceMs`, `maxRetries` — are unaffected and still work. diff --git a/packages/server/src/config.ts b/packages/server/src/config.ts index e2f45d67..a8437167 100644 --- a/packages/server/src/config.ts +++ b/packages/server/src/config.ts @@ -628,7 +628,6 @@ export interface PaddockConfigFile { autoReDrive?: boolean | string; debounceMs?: number | string; maxRetries?: number | string; - limboTimeoutMs?: number | string; }; /** * Inbound composer-attachment config (issue #328). Every field optional; a @@ -1185,11 +1184,6 @@ function loadRecoveryConfig(file?: PaddockConfigFile["recovery"]): RecoveryConfi f.maxRetries, DEFAULT_RECOVERY.maxRetries, ), - limboTimeoutMs: loadRecoveryInt( - "PADDOCK_RECOVERY_LIMBO_MS", - f.limboTimeoutMs, - DEFAULT_RECOVERY.limboTimeoutMs, - ), }; } diff --git a/packages/server/src/instance-config.ts b/packages/server/src/instance-config.ts index 0382e94f..3b45790b 100644 --- a/packages/server/src/instance-config.ts +++ b/packages/server/src/instance-config.ts @@ -329,7 +329,6 @@ export const FIELDS: readonly FieldSpec[] = [ { key: "recovery.autoReDrive", group: "recovery", label: "Auto re-drive", type: "boolean", envVars: ["PADDOCK_RECOVERY_AUTODRIVE"], default: DEFAULT_RECOVERY.autoReDrive, editable: true, coerce: asBool }, { key: "recovery.debounceMs", group: "recovery", label: "Debounce (ms)", type: "number", envVars: ["PADDOCK_RECOVERY_DEBOUNCE_MS"], default: DEFAULT_RECOVERY.debounceMs, editable: true, coerce: nonNegInt }, { key: "recovery.maxRetries", group: "recovery", label: "Max retries", type: "number", envVars: ["PADDOCK_RECOVERY_MAX_RETRIES"], default: DEFAULT_RECOVERY.maxRetries, editable: true, coerce: nonNegInt }, - { key: "recovery.limboTimeoutMs", group: "recovery", label: "Limbo timeout (ms)", type: "number", envVars: ["PADDOCK_RECOVERY_LIMBO_MS"], default: DEFAULT_RECOVERY.limboTimeoutMs, editable: true, coerce: nonNegInt }, // Attachments (issue #328). { key: "attachments.enabled", group: "attachments", label: "Enabled", type: "boolean", envVars: ["PADDOCK_ATTACHMENTS_ENABLED"], default: DEFAULT_ATTACHMENTS.enabled, editable: true, coerce: asBool }, diff --git a/packages/server/src/recovery-config.ts b/packages/server/src/recovery-config.ts index f9d332ab..07bb854e 100644 --- a/packages/server/src/recovery-config.ts +++ b/packages/server/src/recovery-config.ts @@ -68,12 +68,6 @@ export interface RecoveryConfig { * `PADDOCK_RECOVERY_MAX_RETRIES`. */ maxRetries: number; - /** - * Layer 2 backstop — if a kept-alive keeper session shows no activity for this - * many ms after a killed-task notification, surface it as stuck. `0` disables - * the backstop (the default). Env `PADDOCK_RECOVERY_LIMBO_MS`. - */ - limboTimeoutMs: number; } /** @@ -91,7 +85,6 @@ export const DEFAULT_RECOVERY: RecoveryConfig = Object.freeze({ autoReDrive: false, debounceMs: 5000, maxRetries: 1, - limboTimeoutMs: 0, }); /** The `` values a killed-at-turn-boundary task notification carries. */ @@ -128,7 +121,6 @@ export function sanitizeRecoveryOverride(value: unknown): RecoveryOverride | und if (typeof o.autoReDrive === "boolean") out.autoReDrive = o.autoReDrive; if (isNonNegativeInt(o.debounceMs)) out.debounceMs = o.debounceMs; if (isNonNegativeInt(o.maxRetries)) out.maxRetries = o.maxRetries; - if (isNonNegativeInt(o.limboTimeoutMs)) out.limboTimeoutMs = o.limboTimeoutMs; return Object.keys(out).length > 0 ? out : undefined; } @@ -150,6 +142,5 @@ export function resolveRecoveryConfig( autoReDrive: clean.autoReDrive ?? instanceDefault.autoReDrive, debounceMs: clean.debounceMs ?? instanceDefault.debounceMs, maxRetries: clean.maxRetries ?? instanceDefault.maxRetries, - limboTimeoutMs: clean.limboTimeoutMs ?? instanceDefault.limboTimeoutMs, }; } diff --git a/packages/server/test/integration/instance-config.test.ts b/packages/server/test/integration/instance-config.test.ts index ca26ee5a..6e83fa02 100644 --- a/packages/server/test/integration/instance-config.test.ts +++ b/packages/server/test/integration/instance-config.test.ts @@ -135,7 +135,6 @@ describe("integration: instance-config (#385)", () => { patch: { "recovery.debounceMs": null, "recovery.maxRetries": null, - "recovery.limboTimeoutMs": null, }, }); expect(res.statusCode).toBe(200); @@ -143,7 +142,6 @@ describe("integration: instance-config (#385)", () => { const recovery = (await readYaml()).recovery ?? {}; expect(recovery.debounceMs).toBeUndefined(); expect(recovery.maxRetries).toBeUndefined(); - expect(recovery.limboTimeoutMs).toBeUndefined(); // And the screen reports the built-in defaults as pending, not zeros. const fields = flat(await get()); diff --git a/packages/server/test/integration/routes.test.ts b/packages/server/test/integration/routes.test.ts index 92f63d5b..150aeca7 100644 --- a/packages/server/test/integration/routes.test.ts +++ b/packages/server/test/integration/routes.test.ts @@ -84,7 +84,6 @@ describe("integration: REST route coverage (real app, fake claude)", () => { autoReDrive: false, debounceMs: 5000, maxRetries: 1, - limboTimeoutMs: 0, }); // With no PADDOCK_MODELS set, the full catalog is offered (unchanged behaviour): // every catalog id is present + the keeper default is the real default (#457). diff --git a/packages/server/test/unit/config.test.ts b/packages/server/test/unit/config.test.ts index 1cc55119..d993f3ca 100644 --- a/packages/server/test/unit/config.test.ts +++ b/packages/server/test/unit/config.test.ts @@ -201,7 +201,6 @@ describe("loadPaddockConfig: recovery (#301)", () => { "PADDOCK_RECOVERY_AUTODRIVE", "PADDOCK_RECOVERY_DEBOUNCE_MS", "PADDOCK_RECOVERY_MAX_RETRIES", - "PADDOCK_RECOVERY_LIMBO_MS", ]; beforeEach(async () => { @@ -227,7 +226,6 @@ describe("loadPaddockConfig: recovery (#301)", () => { autoReDrive: false, debounceMs: 5000, maxRetries: 1, - limboTimeoutMs: 0, }); }); @@ -244,9 +242,8 @@ describe("loadPaddockConfig: recovery (#301)", () => { it("parses the numeric knobs from env", () => { process.env.PADDOCK_RECOVERY_DEBOUNCE_MS = "1500"; process.env.PADDOCK_RECOVERY_MAX_RETRIES = "3"; - process.env.PADDOCK_RECOVERY_LIMBO_MS = "30000"; const r = loadPaddockConfig().recovery; - expect(r).toMatchObject({ debounceMs: 1500, maxRetries: 3, limboTimeoutMs: 30000 }); + expect(r).toMatchObject({ debounceMs: 1500, maxRetries: 3 }); }); it.each(["-1", "1.5", "nonsense", ""])( @@ -426,7 +423,6 @@ describe("loadPaddockConfig: YAML instance-config file (#270)", () => { " surfaceKilledTask: false", " autoReDrive: true", " debounceMs: 2500", - " limboTimeoutMs: 60000", "gitAuthor:", " name: Ed", " email: ed@example.com", @@ -455,7 +451,6 @@ describe("loadPaddockConfig: YAML instance-config file (#270)", () => { autoReDrive: true, debounceMs: 2500, maxRetries: 1, - limboTimeoutMs: 60000, }); expect(cfg.gitAuthor).toEqual({ name: "Ed", email: "ed@example.com" }); }); diff --git a/packages/server/test/unit/instance-config.test.ts b/packages/server/test/unit/instance-config.test.ts index c306277f..8be7ebe5 100644 --- a/packages/server/test/unit/instance-config.test.ts +++ b/packages/server/test/unit/instance-config.test.ts @@ -230,7 +230,7 @@ describe("instance-config (#385)", () => { // zero: `maxRetries: 0` disables recovery retries, `debounceMs: 0` removes // the debounce. The clear must produce `value: null` (the writer's delete). it("clears a nonNegInt field with null / empty string rather than writing 0 (#723)", () => { - for (const key of ["recovery.debounceMs", "recovery.maxRetries", "recovery.limboTimeoutMs"]) { + for (const key of ["recovery.debounceMs", "recovery.maxRetries"]) { expect(validatePatch({ [key]: null })).toEqual([{ key, value: null }]); expect(validatePatch({ [key]: "" })).toEqual([{ key, value: null }]); } diff --git a/packages/server/test/unit/recovery-config.test.ts b/packages/server/test/unit/recovery-config.test.ts index 8c379e98..ed0af694 100644 --- a/packages/server/test/unit/recovery-config.test.ts +++ b/packages/server/test/unit/recovery-config.test.ts @@ -20,7 +20,6 @@ describe("DEFAULT_RECOVERY", () => { autoReDrive: false, debounceMs: 5000, maxRetries: 1, - limboTimeoutMs: 0, }); }); }); @@ -45,11 +44,16 @@ describe("sanitizeRecoveryOverride", () => { autoReDrive: "yes", // wrong type → dropped debounceMs: 1234, maxRetries: -1, // negative → dropped - limboTimeoutMs: 2.5, // non-integer → dropped }), ).toEqual({ surfaceKilledTask: false, debounceMs: 1234 }); }); + it("drops a non-integer numeric knob", () => { + expect(sanitizeRecoveryOverride({ debounceMs: 2.5, maxRetries: 1 })).toEqual({ + maxRetries: 1, + }); + }); + it("returns undefined for a non-object / empty / all-invalid value", () => { expect(sanitizeRecoveryOverride(undefined)).toBeUndefined(); expect(sanitizeRecoveryOverride(null)).toBeUndefined(); @@ -60,9 +64,9 @@ describe("sanitizeRecoveryOverride", () => { }); it("accepts 0 for the numeric knobs (a valid non-negative integer)", () => { - expect(sanitizeRecoveryOverride({ debounceMs: 0, limboTimeoutMs: 0 })).toEqual({ + expect(sanitizeRecoveryOverride({ debounceMs: 0, maxRetries: 0 })).toEqual({ debounceMs: 0, - limboTimeoutMs: 0, + maxRetries: 0, }); }); }); @@ -73,7 +77,6 @@ describe("resolveRecoveryConfig", () => { autoReDrive: false, debounceMs: 5000, maxRetries: 1, - limboTimeoutMs: 0, }; it("inherits every field from the instance default when there is no override", () => { @@ -88,7 +91,6 @@ describe("resolveRecoveryConfig", () => { autoReDrive: true, debounceMs: 5000, maxRetries: 1, - limboTimeoutMs: 0, }); }); diff --git a/packages/web/src/components/ChatPane.test.tsx b/packages/web/src/components/ChatPane.test.tsx index 5ec954c8..d286a6c9 100644 --- a/packages/web/src/components/ChatPane.test.tsx +++ b/packages/web/src/components/ChatPane.test.tsx @@ -1569,7 +1569,6 @@ describe("ChatPane: killed background-task recovery (#301)", () => { autoReDrive: false, debounceMs: 5000, maxRetries: 1, - limboTimeoutMs: 0, }, }); const loadHistory = vi.fn().mockResolvedValue(killedHistory); diff --git a/packages/web/src/lib/types.ts b/packages/web/src/lib/types.ts index 5a06c33a..01199b92 100644 --- a/packages/web/src/lib/types.ts +++ b/packages/web/src/lib/types.ts @@ -30,8 +30,6 @@ export interface RecoveryConfig { debounceMs: number; /** Layer 3 — per-session auto re-drive retry cap. */ maxRetries: number; - /** Layer 2 backstop — surface a limbo session after N ms of silence (0 = off). */ - limboTimeoutMs: number; } /** A per-project recovery override — every field optional (absent ⇒ inherit). */ diff --git a/packages/web/src/test/factories.ts b/packages/web/src/test/factories.ts index bf614260..f4868a19 100644 --- a/packages/web/src/test/factories.ts +++ b/packages/web/src/test/factories.ts @@ -27,7 +27,6 @@ export function makeModelsResponse(over: Partial = {}): ModelsRe autoReDrive: false, debounceMs: 5000, maxRetries: 1, - limboTimeoutMs: 0, }, attachmentsDefault: { enabled: true, diff --git a/website/src/content/docs/architecture/overview.md b/website/src/content/docs/architecture/overview.md index 1dd63082..0f7bdb3f 100644 --- a/website/src/content/docs/architecture/overview.md +++ b/website/src/content/docs/architecture/overview.md @@ -648,7 +648,7 @@ The main knobs: | **Agent capabilities** | `PADDOCK_BROWSER_MCP` (false — Playwright/headless Chromium, via the static agent config, not injection) | | **Sweeper** | `PADDOCK_SWEEP_MIN_INTERVAL_MS` (300000) | | **Curation budgets** | `PADDOCK_CURATION_OVERVIEW_MAX_TOKENS` (2000), `PADDOCK_CURATION_CHANGELOG_MAX_TOKENS` (8000), `PADDOCK_CURATION_CLAUDEMD_MAX_TOKENS` (6000) — `curation.*` in YAML, per-project overridable (`curation-config.ts`) | -| **Chat recovery** | `PADDOCK_RECOVERY_SURFACE` (**true** — `surfaceKilledTask`, the `chat:killed_task` affordance), `PADDOCK_RECOVERY_AUTODRIVE` (false), `PADDOCK_RECOVERY_DEBOUNCE_MS` (5000), `PADDOCK_RECOVERY_MAX_RETRIES` (1), `PADDOCK_RECOVERY_LIMBO_MS` (0 = off) | +| **Chat recovery** | `PADDOCK_RECOVERY_SURFACE` (**true** — `surfaceKilledTask`, the `chat:killed_task` affordance), `PADDOCK_RECOVERY_AUTODRIVE` (false), `PADDOCK_RECOVERY_DEBOUNCE_MS` (5000), `PADDOCK_RECOVERY_MAX_RETRIES` (1) | | **Attachments** | `PADDOCK_ATTACHMENTS_ENABLED` (true), `PADDOCK_ATTACHMENTS_MAX_FILE_SIZE_MB` (25), `PADDOCK_ATTACHMENTS_MAX_FILES_PER_MESSAGE` (10), `PADDOCK_ATTACHMENTS_ALLOWED_TYPES` (`*` — a hygiene guardrail, **not** a security boundary) | | **OpenAPI** | `PADDOCK_OPENAPI_ENABLED` (false), `PADDOCK_OPENAPI_PATH` (`/open-api`) — see [§12](#12-openapi-reference) | | **Management API** | **YAML-only** `managementApi.*` — `clients[]`, `instanceId`, `trustedProxies` — plus the `PADDOCK_MCP_TOKEN_` credentials and `PADDOCK_MANAGEMENT_TRUSTED_PROXIES`. See [Management API (MCP)](/reference/mcp/). | diff --git a/website/src/content/docs/configuration/chat-recovery.md b/website/src/content/docs/configuration/chat-recovery.md index 33a4cc46..ca4324fc 100644 --- a/website/src/content/docs/configuration/chat-recovery.md +++ b/website/src/content/docs/configuration/chat-recovery.md @@ -187,7 +187,6 @@ Set these in the environment (or the YAML instance-config file under a top-level | `autoReDrive` | `PADDOCK_RECOVERY_AUTODRIVE` | `false` (OFF) | 3 | Automatically re-drive a hung chat (detect the killed task + inject the nudge on its own). | | `debounceMs` | `PADDOCK_RECOVERY_DEBOUNCE_MS` | `5000` | 3 | Quiet window (ms) after a killed task before auto re-drive fires, so a chat that's genuinely finishing isn't poked. | | `maxRetries` | `PADDOCK_RECOVERY_MAX_RETRIES` | `1` | 3 | Per-session cap on auto re-drives, so a wedged chat isn't poked in a loop. | -| `limboTimeoutMs` | `PADDOCK_RECOVERY_LIMBO_MS` | `0` (off) | 2 | If set, surface a kept-alive session as stuck after this many ms of silence following a killed task. `0` disables it. *(Backstop timer ships in a follow-up.)* | Booleans accept `1`/`true`/`yes` (case-insensitive) for on and anything else for off. The numeric knobs accept non-negative integers; an invalid value falls back to @@ -201,7 +200,6 @@ recovery: autoReDrive: false debounceMs: 5000 maxRetries: 1 - limboTimeoutMs: 0 ``` ### Per-project override diff --git a/website/src/content/docs/configuration/environment.md b/website/src/content/docs/configuration/environment.md index e99ad2cc..ca51ab91 100644 --- a/website/src/content/docs/configuration/environment.md +++ b/website/src/content/docs/configuration/environment.md @@ -263,7 +263,6 @@ a per-project `recovery` override in `project.yaml`. | `PADDOCK_RECOVERY_AUTODRIVE` | `false` (OFF) | no | **Layer 3.** Automatically re-drive a hung chat — Paddock detects the killed task and injects the nudge on its own (debounce + retry-cap guarded). Off by default (it acts unattended and costs a turn). | | `PADDOCK_RECOVERY_DEBOUNCE_MS` | `5000` | no | Layer 3: quiet window (ms) after a killed task before auto re-drive fires. Non-negative integer, else the default. | | `PADDOCK_RECOVERY_MAX_RETRIES` | `1` | no | Layer 3: per-session cap on auto re-drives (no poke-loops). Non-negative integer, else the default. | -| `PADDOCK_RECOVERY_LIMBO_MS` | `0` (off) | no | Layer 2 backstop: surface a kept-alive session as stuck after this many ms of silence following a killed task. `0` disables it. *(Backstop timer ships in a follow-up — config only for now.)* | ## Attachments (inbound uploads)