Skip to content

OpenAPI Phase 2: memories route Zod schemas + handler refactor#26

Merged
ethanj merged 2 commits intoarchitecture2from
arch2-openapi-phase2-memories-schemas
Apr 20, 2026
Merged

OpenAPI Phase 2: memories route Zod schemas + handler refactor#26
ethanj merged 2 commits intoarchitecture2from
arch2-openapi-phase2-memories-schemas

Conversation

@ethanj
Copy link
Copy Markdown
Contributor

@ethanj ethanj commented Apr 20, 2026

Summary

Refactors all 24 `/v1/memories/*` handlers to delegate request parsing to Zod schemas via `validateBody` / `validateQuery` / `validateParams`. Deletes the 130-line inline parser block at `memories.ts:515-647` (`parseIngestBody`, `parseSearchBody`, `parseOptionalWorkspaceContext`, etc.).

Contract preservation

Every existing behavior is preserved byte-for-byte:

Contract Preserved how
400 envelope `{ error: string }` `formatZodIssues` flattens ZodError issues to a single string matching prior `InputError` message style
`parseOptionalWorkspaceContext` silent-drop-partial `WorkspaceIdField` / `AgentIdField` / `VisibilityField` with `.catch(undefined)`
`parseOptionalAgentScope` silent-drop-invalid `AgentScopeSchema.optional().catch(undefined)`
`parseOptionalIsoTimestamp` `'' / null` = absent `IsoTimestamp` preprocess step
`retrieval_mode` error messages Custom `superRefine` emits exact prior-parser strings
`token_budget` `[100, 50000]` floor Preserved
`conversation` 100k max Preserved with exact error message
PUT `/config` 410 + `rejected[]` Stays in-handler (business logic, not input validation)

Middleware fix

Express 5 exposes `req.query` / `req.params` as getter-only properties on `Request.prototype`. Plain `req.query = parsed` silently no-ops. Validators now use `Object.defineProperty` to shadow the prototype accessor with an own property so handlers see the parsed+transformed value.

Test plan

  • `npx tsc --noEmit` — clean
  • `npm test` — 1009/1009 tests pass
  • `npx fallow --no-cache` — exit 0
  • `memories.ts` shrank 768 → 667 LOC (inline helpers removed)

ethanj added 2 commits April 20, 2026 00:49
Refactors all 24 /v1/memories/* handlers to delegate request parsing
to Zod schemas via validateBody / validateQuery / validateParams.
Deletes the inline parseIngestBody / parseSearchBody /
parseOptionalWorkspaceContext / parseOptionalAgentScope /
parseOptionalIsoTimestamp / parseTokenBudget / parseRetrievalMode /
requireBodyString / requireQueryString / requireUuidParam /
optionalBodyString / optionalQueryString / optionalUuidQuery / parseLimit /
parseUserIdAndLimit helpers (previously 515-647 in memories.ts).

Every request contract is preserved byte-for-byte:
  - 400 envelope is { error: string } (formatZodIssues flattens the
    ZodError issue list into the same style as the prior InputError
    messages).
  - parseOptionalWorkspaceContext's silent-drop-partial contract is
    preserved via WorkspaceIdField / AgentIdField / VisibilityField.
  - parseOptionalAgentScope's silent-drop-invalid contract is
    preserved via AgentScopeSchema.optional().catch(undefined).
  - parseOptionalIsoTimestamp's '' / null = absent contract preserved
    via IsoTimestamp's preprocess step.
  - retrieval_mode / token_budget / conversation-length / limit-clamp
    math matches the original parsers exactly.
  - PUT /config's 410 + rejected[] handler logic stays in-handler
    (those are business rules, not input validation).

Middleware fix: Express 5 exposes req.query / req.params as
getter-only properties on Request.prototype. The validators now use
Object.defineProperty to shadow the prototype accessor with an own
property, so handlers see the parsed + transformed values after
validateQuery / validateParams. req.body is writable normally.

1009/1009 tests pass. tsc clean. fallow exit 0. memories.ts shrank
768 → 667 LOC (inline helpers removed).
…lters

Codex review caught two byte-level wire-contract regressions on the
Phase 2 refactor.

1. Required-field error messages leaked generic Zod text. Plain
   z.string().min(1) for user_id / conversation / source_site /
   query / pattern / memory_ids produced "Invalid input: expected
   string, received undefined" on missing/wrong-type inputs, while
   the old parsers emitted "${label} (string) is required" /
   "memory_ids (string[]) is required". Clients matching on the
   exact message would have broken.

   Fix: add requiredStringBody(label) and requiredStringArrayBody(
   label) helpers that build schemas whose single refine() produces
   the exact prior-parser message for EVERY failure mode (missing,
   null, wrong type, empty string). Applied to every required body
   field across IngestBody, SearchBody, ExpandBody, ConsolidateBody,
   DecayBody, ResetSourceBody, LessonReportBody.

2. source_site / namespace_scope empty-string collapsed to undefined.
   The Phase 2 OptionalBodyString transform dropped '' to undefined;
   optionalBodyString() preserved it verbatim. POST /search with
   source_site: '' or namespace_scope: '' no longer reached the
   service with the same payload.

   Fix: redefine OptionalBodyString as `z.string().optional().catch(
   undefined)` — no length transform. Empty string passes through
   unchanged; non-strings and null coerce to undefined.

Regression tests in src/schemas/__tests__/memories.test.ts pin both:
   - Exact "${label} (string) is required" / "(string[]) is
     required" messages across missing / non-string / empty-string /
     non-array inputs.
   - source_site: '' and namespace_scope: '' round-trip verbatim.
   - Over-length conversation error message matches verbatim.

1023/1023 tests pass; tsc clean; fallow exit 0.
@ethanj ethanj merged commit 5f0b480 into architecture2 Apr 20, 2026
@ethanj ethanj deleted the arch2-openapi-phase2-memories-schemas branch April 20, 2026 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant