refactor: parse vendor and Json boundaries in the agent lib (CMP-82) - #149
refactor: parse vendor and Json boundaries in the agent lib (CMP-82)#149ripgrim wants to merge 2 commits into
Conversation
…validation The agent version manifest is written in one package and read in three. Each reader had its own recordOf helper walking the same JSON column, so the stored shape was described four times and agreed on nowhere. It now lives in packages/validation beside the schemas that were already there, with the parse helper that already existed. Every reader consumes that one definition and the private helpers are gone. Trigger config and the CRM event payload move for the same reason: written by the API, read by the agent. Values are imported by subpath rather than the barrel. A value re-export from index.ts trips noBarrelFile, and the subpath keeps the db and slack schemas out of the client bundle. Rows written before this change still load. Where the old code tolerated bad input it still tolerates it, to the same fallback; where it threw it still throws, with the same message. Each path was checked against the original with padded strings, Infinity, NaN, fractional intervals and arrays. The review-version manifest now crosses tRPC parsed rather than raw. That was forced: reading the property off the generated output type raises TS2589, which is why the client had cast through unknown to reach it. Parsing server-side removes both casts and renders identically.
Every vendor response is now parsed where it arrives, so nothing downstream sees the raw shape. LinkedIn, GitHub, Slack and the Context extract each gain a schema at their fetch, and the private str/int and recordOf helpers that stood in for one are gone. The extract schema was itself a Record<string, unknown>, which is the thing the boundary was supposed to prevent. It is a JsonSchema now, and the team-page payload is parsed by its owner - the caller supplying the schema is the only code that knows the shape. Prisma Json columns are read as Prisma.JsonValue and parsed with a named schema rather than walked with typeof. Behaviour is unchanged. Every vendor schema catches at each level, so a malformed response still degrades to null or an empty list exactly as the typeof guards did; docs/agent.md requires a missing key to remove a capability and never throw. Where the old code threw it still throws, on the same condition and with the same message. Error helpers keep unknown and are renamed to cause. A catch binding is unknown by language rule and cannot be schema'd; cause is the documented exemption and error-formatter.ts already set that precedent. HOST_ALIASES became a Map, which closes a latent prototype-key hole: http://constructor/ would have stringified Object.prototype.constructor into a canonical value.
|
@ripgrim is attempting to deploy a commit to the Comp AI - PoC Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
5 issues found across 39 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/validation/src/agent-manifest.ts">
<violation number="1" location="packages/validation/src/agent-manifest.ts:136">
P2: A persisted schedule config with `intervalMinutes: 1.5` passes `readAgentTriggerConfig` and produces fractional-minute schedules. Add integer validation here to keep this boundary consistent with `agentScheduleTriggerConfig` and the builder input schema.</violation>
<violation number="2" location="packages/validation/src/agent-manifest.ts:147">
P2: readAgentTriggerConfig returns UNREADABLE_TRIGGER_CONFIG (intervalMinutes 1440, event null) whenever the stored config fails safeParse. This collapses 'unreadable config' into 'absent config', contradicting the rule documented in AGENTS.md for this exact module ('Parse failure is a real error... Do not swallow it into an empty array, because unreadable manifest and no actions are different problems'). In custom-agent-dispatch.ts:251 a SCHEDULE trigger with an unreadable config silently reschedules at 1440 minutes (daily) instead of surfacing the bad config, and at line 341 an EVENT trigger with an unreadable config returns event: null and silently never matches. Because agentTriggerConfig has no strict-total marker, an object missing intervalMinutes also parses success with fallback 1440, so missing and unreadable are indistinguishable. Consider failing loudly (or returning a distinguishable sentinel) instead of a silently acting default.</violation>
</file>
<file name="apps/agent/agent/lib/socials.ts">
<violation number="1" location="apps/agent/agent/lib/socials.ts:22">
P2: When GitHub returns a 2xx non-object body, this object-level catch fabricates a user-shaped result and can persist a handle-only suggestion. Let top-level account parsing fail into the surrounding error path instead of returning an all-null account.</violation>
</file>
<file name="apps/agent/agent/lib/linkdapi.ts">
<violation number="1" location="apps/agent/agent/lib/linkdapi.ts:22">
P1: When the API returns only one of the legacy experience keys, this schema discards the entire work history because the other key is required. Make both keys optional so either response shape is preserved.</violation>
</file>
<file name="packages/validation/src/agent-events.ts">
<violation number="1" location="packages/validation/src/agent-events.ts:19">
P2: The outer .refine rejects a task whose record.kind does not match the event's recordKind, but it passes no message. Zod emits a generic 'Invalid input' error with an empty path, and the consumer throws 'The queued agent event is invalid.' which discards schema detail. A failed parse gives no indication whether type, record.id, occurredAt, or recordKind mismatched, so an invalid queued event is effectively undiagnosable. Pass a message describing the expected recordKind for the event type.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
| .catch({ success: null, data: null }); | ||
|
|
||
| const experienceEnvelope = z | ||
| .object({ experience: rows, experiences: rows }) |
There was a problem hiding this comment.
P1: When the API returns only one of the legacy experience keys, this schema discards the entire work history because the other key is required. Make both keys optional so either response shape is preserved.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/agent/agent/lib/linkdapi.ts, line 22:
<comment>When the API returns only one of the legacy experience keys, this schema discards the entire work history because the other key is required. Make both keys optional so either response shape is preserved.</comment>
<file context>
@@ -1,6 +1,33 @@
+ .catch({ success: null, data: null });
+
+const experienceEnvelope = z
+ .object({ experience: rows, experiences: rows })
+ .catch({ experience: null, experiences: null });
+
</file context>
| .object({ experience: rows, experiences: rows }) | |
| \t.object({ experience: rows.optional(), experiences: rows.optional() }) |
| @@ -1,6 +1,20 @@ | |||
| import { CRM_EVENT_TYPES } from "@crm/db/crm-events"; | |||
There was a problem hiding this comment.
P2: readAgentTriggerConfig returns UNREADABLE_TRIGGER_CONFIG (intervalMinutes 1440, event null) whenever the stored config fails safeParse. This collapses 'unreadable config' into 'absent config', contradicting the rule documented in AGENTS.md for this exact module ('Parse failure is a real error... Do not swallow it into an empty array, because unreadable manifest and no actions are different problems'). In custom-agent-dispatch.ts:251 a SCHEDULE trigger with an unreadable config silently reschedules at 1440 minutes (daily) instead of surfacing the bad config, and at line 341 an EVENT trigger with an unreadable config returns event: null and silently never matches. Because agentTriggerConfig has no strict-total marker, an object missing intervalMinutes also parses success with fallback 1440, so missing and unreadable are indistinguishable. Consider failing loudly (or returning a distinguishable sentinel) instead of a silently acting default.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/validation/src/agent-manifest.ts, line 147:
<comment>readAgentTriggerConfig returns UNREADABLE_TRIGGER_CONFIG (intervalMinutes 1440, event null) whenever the stored config fails safeParse. This collapses 'unreadable config' into 'absent config', contradicting the rule documented in AGENTS.md for this exact module ('Parse failure is a real error... Do not swallow it into an empty array, because unreadable manifest and no actions are different problems'). In custom-agent-dispatch.ts:251 a SCHEDULE trigger with an unreadable config silently reschedules at 1440 minutes (daily) instead of surfacing the bad config, and at line 341 an EVENT trigger with an unreadable config returns event: null and silently never matches. Because agentTriggerConfig has no strict-total marker, an object missing intervalMinutes also parses success with fallback 1440, so missing and unreadable are indistinguishable. Consider failing loudly (or returning a distinguishable sentinel) instead of a silently acting default.</comment>
<file context>
@@ -109,3 +130,56 @@ export function parseAgentManifest(value: unknown): AgentManifest {
+
+export type AgentTriggerConfig = z.infer<typeof agentTriggerConfig>;
+
+const UNREADABLE_TRIGGER_CONFIG: AgentTriggerConfig = {
+ intervalMinutes: AGENT_TRIGGER_INTERVAL_MINUTES.fallback,
+ event: null,
</file context>
|
|
||
| export const agentTriggerConfig = z.object({ | ||
| intervalMinutes: z | ||
| .number() |
There was a problem hiding this comment.
P2: A persisted schedule config with intervalMinutes: 1.5 passes readAgentTriggerConfig and produces fractional-minute schedules. Add integer validation here to keep this boundary consistent with agentScheduleTriggerConfig and the builder input schema.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/validation/src/agent-manifest.ts, line 136:
<comment>A persisted schedule config with `intervalMinutes: 1.5` passes `readAgentTriggerConfig` and produces fractional-minute schedules. Add integer validation here to keep this boundary consistent with `agentScheduleTriggerConfig` and the builder input schema.</comment>
<file context>
@@ -109,3 +130,56 @@ export function parseAgentManifest(value: unknown): AgentManifest {
+
+export const agentTriggerConfig = z.object({
+ intervalMinutes: z
+ .number()
+ .min(AGENT_TRIGGER_INTERVAL_MINUTES.min)
+ .transform((minutes) =>
</file context>
| bio: text, | ||
| type: rawText, | ||
| }) | ||
| .catch({ |
There was a problem hiding this comment.
P2: When GitHub returns a 2xx non-object body, this object-level catch fabricates a user-shaped result and can persist a handle-only suggestion. Let top-level account parsing fail into the surrounding error path instead of returning an all-null account.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/agent/agent/lib/socials.ts, line 22:
<comment>When GitHub returns a 2xx non-object body, this object-level catch fabricates a user-shaped result and can persist a handle-only suggestion. Let top-level account parsing fail into the surrounding error path instead of returning an all-null account.</comment>
<file context>
@@ -6,6 +7,27 @@ import {
+ bio: text,
+ type: rawText,
+ })
+ .catch({
+ login: null,
+ name: null,
</file context>
| data: z.record(z.string(), z.json()).catch({}), | ||
| }) | ||
| .refine( | ||
| (task) => CRM_EVENT_CATALOG[task.type].recordKind === task.record.kind, |
There was a problem hiding this comment.
P2: The outer .refine rejects a task whose record.kind does not match the event's recordKind, but it passes no message. Zod emits a generic 'Invalid input' error with an empty path, and the consumer throws 'The queued agent event is invalid.' which discards schema detail. A failed parse gives no indication whether type, record.id, occurredAt, or recordKind mismatched, so an invalid queued event is effectively undiagnosable. Pass a message describing the expected recordKind for the event type.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/validation/src/agent-events.ts, line 19:
<comment>The outer .refine rejects a task whose record.kind does not match the event's recordKind, but it passes no message. Zod emits a generic 'Invalid input' error with an empty path, and the consumer throws 'The queued agent event is invalid.' which discards schema detail. A failed parse gives no indication whether type, record.id, occurredAt, or recordKind mismatched, so an invalid queued event is effectively undiagnosable. Pass a message describing the expected recordKind for the event type.</comment>
<file context>
@@ -0,0 +1,22 @@
+ data: z.record(z.string(), z.json()).catch({}),
+ })
+ .refine(
+ (task) => CRM_EVENT_CATALOG[task.type].recordKind === task.record.kind,
+ );
+
</file context>
Second in the sequence. 524 → 419 repo-wide;
apps/agent/agent/lib/goes 105 → 0, across 24 files.What changed
Vendor responses are parsed where they arrive. LinkedIn, GitHub, Slack and the Context extract each gain a schema at their
fetch, so nothing downstream ever sees the raw shape. The privatestr/int/recordOfhelpers that were standing in for a parser are deleted, not relocated.The extract schema was itself a
Record<string, unknown>— the exact thing a boundary is meant to prevent. It is a real recursiveJsonSchemanow, and the team-page payload is parsed by its owner: the caller supplying the schema is the only code that knows the shape.Prisma
Jsoncolumns are read asPrisma.JsonValueand parsed with a named schema rather than walked withtypeof.Behaviour is unchanged, deliberately
Every vendor schema
.catch()es at each level, so a malformed response still degrades tonull/[]exactly as the oldtypeofguards did.docs/agent.mdrequires that a missing key removes a capability and never throws, and that still holds. Where the old code threw, it throws on the same condition with the same message.Two things worth calling out:
unknown, renamed tocause. Acatchbinding isunknownby language rule and cannot be schema'd;causeis the rule's own documented exemption, andapps/api/src/trpc/error-formatter.tsalready set that precedent.extract()still returnsdata: unknown, on purpose. The caller supplies the JSON Schema, so only the caller knows the shape. Tightening it centrally would be a lie;portrait-sources.tsparses it at the point of use instead.One incidental fix
HOST_ALIASESbecame aMap, which closes a latent prototype-key hole:http://constructor/would previously have stringifiedObject.prototype.constructorinto a canonical value.Two edits outside the module, both forced by a signature change
agent/channels/crm.ts— one import and one call, sincefinishRun'sresultis now a parsedRunResult.agent_runner/tools/finish_run.ts— one token,z.unknown()→z.json(), so the tool's inferred type still matches. Tool input is always parsed JSON, so this rejects nothing that could previously arrive.Verification
bun run check-types13/13 ·bun run lint9/9apps/agent313 pass / 0 failapps/agent/agent/lib/at 0; no file outside the module increasedas unknown asadded anywhere in the diff🤖 Generated with Claude Code