diff --git a/docs/api-reference/veryfront/agent.md b/docs/api-reference/veryfront/agent.md index fbff24bb20..15f98382ff 100644 --- a/docs/api-reference/veryfront/agent.md +++ b/docs/api-reference/veryfront/agent.md @@ -615,7 +615,7 @@ Input delivered to a hosted agent-service detached execution callback. | `createAgentServiceChildMirrorContext` | Context for create hosted child mirror. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/agent/hosted/child-mirror.ts#L233) | | `createAgentServiceFormInputTool` | Create hosted form input tool. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/agent/hosted/form-input-tool.ts#L34) | | `createAgentServiceProjectSteering` | Create hosted agent project steering. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/agent/hosted/agent-project-steering.ts#L82) | -| `createAgentServiceRegistrationLifecycle` | Create agent service registration lifecycle. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/agent/service/registration.ts#L501) | +| `createAgentServiceRegistrationLifecycle` | Create agent service registration lifecycle. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/agent/service/registration.ts#L520) | | `createAgentServiceRouteSet` | Create hosted agent service route set. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/agent/service/routes.ts#L214) | | `createAgentServiceRuntime` | Create agent service runtime. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/agent/service/runtime.ts#L233) | | `createAgentServiceServerRuntime` | Create agent service server runtime. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/agent/service/server.ts#L54) | diff --git a/src/agent/service/registration.test.ts b/src/agent/service/registration.test.ts index 76f202913f..0f25748165 100644 --- a/src/agent/service/registration.test.ts +++ b/src/agent/service/registration.test.ts @@ -2,6 +2,7 @@ import "#veryfront/schemas/_test-setup.ts"; import { assert, assertEquals, assertRejects } from "#veryfront/testing/assert.ts"; import { describe, it } from "#veryfront/testing/bdd.ts"; import { waitFor } from "#veryfront/testing"; +import { NETWORK_ERROR } from "#veryfront/errors"; import { type AgentServiceRegistrationLogger, createAgentServiceRegistrationLifecycle, @@ -634,6 +635,91 @@ describe("agent/agent-service-registration heartbeat retry", () => { ); }); + it("fails a heartbeat whose body does not match the schema, without retrying", async () => { + // A body that arrived intact but does not parse is a permanent protocol + // mismatch. Retrying it spends all three attempts of every tick on a + // response that will never parse, and buys nothing: escalation still waits + // for three failed ticks either way. This is the counterpart to the test + // above, which pins that a body read that *fails* is retried. The two + // together are what keep the transport wrapper off the schema parse. + let heartbeatRequests = 0; + const log = recordingLogger(); + + const fetch: typeof globalThis.fetch = (input) => { + if (!input.toString().endsWith("/heartbeat")) { + return Promise.resolve(jsonResponse(serviceResponse)); + } + heartbeatRequests++; + // HTTP 200, valid JSON, wrong shape. + return Promise.resolve(jsonResponse({ nope: true } as never)); + }; + + const lifecycle = await createAgentServiceRegistrationLifecycle( + lifecycleOptions(fetch, { logger: log.logger }), + ); + + await assertRejects(() => lifecycle.heartbeat(), Error); + lifecycle.stop(); + + assertEquals( + heartbeatRequests, + 1, + "a body that fails the schema must fail on the first attempt, with no retry", + ); + assertEquals( + log.warnings.length, + 0, + `a permanent schema mismatch must not log a retry notice, saw ` + + `${log.warnings.map((entry) => entry.message).join(", ")}`, + ); + }); + + it("keeps a fetch rejection that already carries an HTTP status out of the retry loop", async () => { + // `fetch` is a public option, so a caller can supply a transport that + // rejects with an error of ours that is already classified. Its httpStatus + // is what keeps a 4xx from being retried, and rewrapping the rejection as a + // bare transport failure would throw that status away and retry it. + let heartbeatRequests = 0; + const log = recordingLogger(); + + const fetch: typeof globalThis.fetch = (input) => { + if (!input.toString().endsWith("/heartbeat")) { + return Promise.resolve(jsonResponse(serviceResponse)); + } + heartbeatRequests++; + return Promise.reject( + NETWORK_ERROR.create({ + detail: "upstream rejected the heartbeat", + context: { httpStatus: 404 }, + }), + ); + }; + + const lifecycle = await createAgentServiceRegistrationLifecycle( + lifecycleOptions(fetch, { logger: log.logger }), + ); + + const error = await assertRejects(() => lifecycle.heartbeat(), Error); + lifecycle.stop(); + + assertEquals( + (error as { context?: { httpStatus?: unknown } }).context?.httpStatus, + 404, + "the classification the caller's fetch supplied must survive", + ); + assertEquals( + heartbeatRequests, + 1, + "a rejection that already carries a 4xx must not be retried", + ); + assertEquals( + log.warnings.length, + 0, + `a classified client error must not log a retry notice, saw ` + + `${log.warnings.map((entry) => entry.message).join(", ")}`, + ); + }); + it("cancels a pending retry backoff when the lifecycle stops", async () => { let heartbeatAttempts = 0; const fetch: typeof globalThis.fetch = (input) => { diff --git a/src/agent/service/registration.ts b/src/agent/service/registration.ts index bd8f3061e9..d2ea2f5dd0 100644 --- a/src/agent/service/registration.ts +++ b/src/agent/service/registration.ts @@ -369,7 +369,23 @@ async function readAgentPushRuntimeServiceResponse( }); } - const parsed = agentPushRuntimeServiceResponseSchema.parse(await response.json()); + let payload: unknown; + try { + payload = await response.json(); + } catch (cause) { + // The headers landed but the body did not: the deadline fired while the + // JSON was still arriving, or the connection reset mid-body. No complete + // response came back and nothing upstream was applied, so this is as + // transport-level as a failed connect and gets the same retries. It + // carries no httpStatus, which is what marks it retryable. + throw NETWORK_ERROR.create({ detail: getErrorMessage(cause), cause }); + } + + // Outside that wrapper on purpose. A body that arrived intact but does not + // match the schema is a permanent protocol mismatch, not a transient one. + // Wrapping it as a transport error would make every tick spend all three + // attempts on a response that will never parse. + const parsed = agentPushRuntimeServiceResponseSchema.parse(payload); return parsed.service; } @@ -421,27 +437,30 @@ async function sendHeartbeatRequest( fetchImpl: typeof globalThis.fetch, abortSignal: AbortSignal | undefined, ): Promise { + let response: Response; try { - const response = await fetchImpl(getHeartbeatEndpoint(input.apiUrl, input.serviceId), { + response = await fetchImpl(getHeartbeatEndpoint(input.apiUrl, input.serviceId), { method: "POST", headers: createHeaders(input.authToken), signal: abortSignal, }); - return await readAgentPushRuntimeServiceResponse(response); } catch (cause) { - // An error that is already ours is already classified: a non-ok response - // carries its httpStatus, and that status is what keeps a 4xx from being - // retried. Rethrow it untouched. + // A caller-supplied fetch may reject with an error that is already ours, + // and that error already carries its own slug and httpStatus. Reclassifying + // it would drop the status that keeps a 4xx from being retried. if (isVeryfrontError(cause)) throw cause; - // Anything else is transport-level: the connect failed, or the deadline - // fired while the body was still arriving after the headers landed. Either - // way no complete response came back and nothing upstream was applied. It - // carries no httpStatus, which is what marks it retryable below. + // Anything else means no response, so the request never reached a handler + // and applied nothing. It carries no httpStatus, which is what marks it + // transport-level below. throw NETWORK_ERROR.create({ detail: getErrorMessage(cause), cause, }); } + // Deliberately outside the wrapper above. The read does its own transport + // mapping, so a failed body read is still retried, while a non-ok status + // keeps its httpStatus and a schema mismatch stays permanent. + return await readAgentPushRuntimeServiceResponse(response); } /** Upstream response status recorded on a heartbeat failure, if it got one. */