Skip to content

fix(agreements): enforce canonical UUID agreementId (revert #111 relaxation)#132

Closed
important-new wants to merge 10 commits into
InspectorHub:mainfrom
important-new:fix/enforce-uuid-ids
Closed

fix(agreements): enforce canonical UUID agreementId (revert #111 relaxation)#132
important-new wants to merge 10 commits into
InspectorHub:mainfrom
important-new:fix/enforce-uuid-ids

Conversation

@important-new

Copy link
Copy Markdown
Contributor

Why

#111 relaxed the hub send-agreement agreementId from .uuid() to .min(1), reasoning that "agreements.id is TEXT and may hold non-UUID imported/seeded rows." On verification that premise is false for primary keys:

  • agreements.id / inspections.id are always crypto.randomUUID() — every create site (createAgreement, starter-content seed, inspection.service) generates a canonical UUID.
  • The Spectora import never creates non-UUID PKsfreshId('ri'/'rd'/...) (prefix_externalId) is only for template-internal item/section ids inside the template JSON; absent an external id it uses randomUUID().
  • The only non-UUID ids that ever existed were test seeds.

So the relaxation was an unnecessary compat shim accommodating a test fixture — against this project's pre-launch no-compat principle (enforce the canonical format now, while there's no mass user base, rather than tolerate malformed ids).

Change

  • inspection.schema.ts: agreementId back to z.string().uuid().
  • Flip the regression test to assert a non-UUID agreementId is rejected (400) — canonical UUID enforced.

(Companion: PR #131, which would have spread the same relaxation to the Track I-a admin send, was closed; main's admin.schema.ts keeps .uuid().)

Testing

Full unit suite green (1537); type-check:api 0; pre-commit gates pass. No production data is affected — all live ids are already canonical UUIDs.

🤖 Generated with Claude Code

important-new and others added 10 commits June 6, 2026 21:51
The DB stores dates as ISO datetimes (e.g. '2026-05-29T09:00:00'). This
value was loaded verbatim into the form, causing two problems:
1. <input type='date'> could not parse it (showed empty placeholder)
2. sanitizeSettingsPatch bypassed the date→ISO conversion, so the API
   received a datetime without a timezone suffix, failing z.string().datetime()

Fix: strip the time component on load so the date picker gets 'YYYY-MM-DD',
which sanitizeSettingsPatch then correctly converts to a Z-suffixed ISO string.
Applied to closingDate too for consistency.
api.team.members returns {data:{members:[...]}} — the loader was reading
body.data (an object) and calling .filter() on it, causing a TypeError crash.
Align with the BFF sheet route which correctly reads data?.members.
…ings BFF

pageSize: "200" fails Zod refine (valid: 12/25/50/100); the .catch(() => null)
silently swallowed the 400 so the template dropdown was always empty.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…lates & dashboard

- TemplateCombobox: new reusable combobox with debounced search, cursor
  pagination (25/page), keyboard nav, load-more; replaces <select> in
  InspectionSettingsSheet (also fixes pageSize 200->100 Zod refine bug)
- /resources/template-search BFF: token-relay loader for TemplateCombobox
- templates.tsx: upgraded from client-side useMemo to URL-based server-side
  search (?q=) with 350ms debounce; sort remains client-side
- server: listTemplates() gains optional q param with LIKE filter;
  listTemplatesRoute schema extended with q; exactOptionalPropertyTypes fix
- /resources/inspection-search BFF: full cross-all-inspections search
  via GET /api/inspections with search+cursor params
- dashboard.tsx: searchQuery now triggers server-side BFF fetch (300ms
  debounce) instead of client-side bucket filter; load-more for results

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…Hub#111 relaxation)

Production agreements.id / inspections.id are ALWAYS crypto.randomUUID() at
every create site; the Spectora import preserves external ids only for
template-internal items, never as a PK. The only non-UUID ids were test seeds.
So InspectorHub#111's .uuid()->.min(1) relaxation (hub send-agreement) was an unnecessary
compat shim accommodating a test seed, against the project's pre-launch
no-compat principle. Re-tighten to .uuid() and flip the regression test to
assert non-UUID ids are rejected (canonical format enforced before launch).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@important-new

Copy link
Copy Markdown
Contributor Author

Superseded by #133 — the canonical-UUID enforcement commit (35f4d08) is folded into the combined Track J PR. Closing this standalone PR.

@important-new important-new deleted the fix/enforce-uuid-ids branch June 9, 2026 02:52
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