Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/api-reference/veryfront/agent.md
Original file line number Diff line number Diff line change
Expand Up @@ -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) |
Expand Down
86 changes: 86 additions & 0 deletions src/agent/service/registration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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) => {
Expand Down
39 changes: 29 additions & 10 deletions src/agent/service/registration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Wrap schema mismatches in a non-retryable registered error

When a heartbeat returns HTTP 200 with valid but wrong-shaped JSON, this parse now exposes a raw schema-library error through the public heartbeat() promise. Consumers that handle failures by Veryfront error slug or serialize them through the standard error boundary therefore lose the stable classification that the parent implementation provided. Wrap the parse failure in a registered, non-retryable VeryfrontError instead, which preserves the intended single attempt without leaking the validator's error type.

AGENTS.md reference: AGENTS.md:L256-L258

Useful? React with 👍 / 👎.

return parsed.service;
}

Expand Down Expand Up @@ -421,27 +437,30 @@ async function sendHeartbeatRequest(
fetchImpl: typeof globalThis.fetch,
abortSignal: AbortSignal | undefined,
): Promise<AgentPushRuntimeServiceRest> {
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. */
Expand Down
Loading