Open
Conversation
There was a problem hiding this comment.
Pull request overview
Adds shared schema/type definitions for the delivery agent to the @workspace/agent-types package and updates the delivery agent to reuse the shared input schema for request validation.
Changes:
- Added
deliveryConfigSchema,deliveryInputSchema, anddeliveryOutputSchema(plus inferred TS types) to@workspace/agent-types, and re-exported them from the package index. - Added Vitest tests for the new delivery schemas in
@workspace/agent-types. - Updated the delivery agent POST handler to validate request bodies using the shared
deliveryInputSchemaand to return a 400 with details on Zod validation errors.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Adds workspace linkage for @workspace/agent-types in the delivery agent importer. |
| packages/agent-types/src/index.ts | Re-exports the new delivery schemas/types from @workspace/agent-types. |
| packages/agent-types/src/delivery.ts | Introduces Zod schemas and inferred types for delivery config/input/output. |
| packages/agent-types/src/delivery.test.ts | Adds Vitest coverage for the new delivery schemas. |
| apps/agents/delivery/src/_main.ts | Switches request validation to shared schema; adds ZodError→400 response handling; adds docstring. |
| apps/agents/delivery/package.json | Adds @workspace/agent-types dependency. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (2)
apps/agents/delivery/src/_main.ts:33
context.req.json()will throw on invalid JSON, but that currently falls through to the generic 500 handler. Since you’re already returning 400 for Zod validation errors, it would be more correct to also return a 400 for JSON parse errors (e.g., SyntaxError) so malformed requests aren’t reported as internal server errors.
try {
const body = await context.req.json();
const data = await deliveryInputSchema.parseAsync(body);
const token = context.req.header("Authorization");
const deliveryData = await fetchDeliveryDataFromAgentDataAPI(
apps/agents/delivery/src/_main.ts:95
- The new docstring says this function can return
null, and the handler has aif (!deliveryData) { ... skipped: true }branch, but the implementation currently never returnsnull(it throws on any non-OK response). Either implement a realnullreturn for the “no newsletter” case (e.g., 404 from Agent Data API) or update the return type/doc + remove the unreachable skip branch.
/**
* Fetches the delivery data (newsletter and subscribers) for a specific ticker from the Agent Data API.
*
* @param token - The authorization token to pass to the API.
* @param tickerId - The unique identifier of the ticker.
* @returns A promise that resolves to the delivery data containing the newsletter and subscribers, or null.
*/
async function fetchDeliveryDataFromAgentDataAPI(
token: string | undefined,
tickerId: string,
): Promise<{
newsletter: { subject: string; content: string };
subscribers: { email: string }[];
} | null> {
const url = new URL(env.AGENT_DATA_API_URL);
url.pathname = "/api/delivery";
url.searchParams.set("tickerId", tickerId);
const res = await got.get(url.toString(), {
headers: { ...(token && { Authorization: token }) },
throwHttpErrors: false,
});
if (!res.ok) {
throw new Error(`Agent data API error: ${res.statusCode}`);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR
delivery-agentconfig #71How to test