Skip to content

feat(inquiry): redesign status lifecycle, add handler registry, fix runtime bugs - #4

Merged
agreenspan merged 37 commits into
mainfrom
feat/inquiry-system-redesign
Mar 5, 2026
Merged

feat(inquiry): redesign status lifecycle, add handler registry, fix runtime bugs#4
agreenspan merged 37 commits into
mainfrom
feat/inquiry-system-redesign

Conversation

@agreenspan

Copy link
Copy Markdown
Contributor

Summary

  • Schema: replaces resolved/acknowledged with explicit approved, denied, changesRequested statuses — terminal decisions are now first-class enum values, not hidden inside a JSON outcome field
  • Handler registry: new handlers/ folder with per-type subfolders (contentSchema, resolutionSchema, handleApprove, validate) and a central registry — adding a new inquiry type is additive with no changes to shared code
  • Bug fixes: resolution.ts was silently no-oping on every approved invite (wrong type names), targetModel was never set on create (breaking resolve access checks), canceled was incorrectly accepted as a resolve outcome, searchableFields referenced non-existent schema fields

New structure

handlers/
  types.ts                          InquiryHandler type
  index.ts                          registry (InquiryType → handler)
  inviteOrganizationUser/           fully implemented
    contentSchema.ts
    resolutionSchema.ts
    handleApprove.ts
    validate.ts
    index.ts                        unique: true
  createSpace|updateSpace|transferSpace/  typed stubs
services/utils/
  assertUniqueInquiry.ts            409 if open inquiry exists between same parties
  resolveContent.ts                 merges resolution overrides onto content (target can modify terms)

New endpoint

POST /inquiries/:id/request-changes — target only, transitions to changesRequested, stores explanation in resolution JSON

Test plan

  • Create inviteOrganizationUser inquiry → status sent
  • Resolve with approvedOrganizationUser created, status approved
  • Resolve with approved + modified role in body → override applied
  • Attempt second invite same org+user while first is pending → 409
  • Cancel as source → status canceled
  • Target calls request-changes → status changesRequested
  • Source resubmits (update + send) → target can resolve again

🤖 Generated with Claude Code

…untime bugs

Schema:
- Replace `resolved`/`acknowledged` with explicit `approved`, `denied`, `changesRequested` statuses
- Status now encodes the terminal decision directly — no redundant outcome field

Handler registry (apps/api/src/modules/inquiry/handlers/):
- Per-type folders with contentSchema, resolutionSchema, handleApprove, validate
- Registry maps InquiryType → InquiryHandler, eliminating switch statements
- inviteOrganizationUser: fully implemented with membership guard and unique enforcement
- createSpace / updateSpace / transferSpace: typed stubs with unique flags

New utils:
- assertUniqueInquiry: 409 if open inquiry already exists between same parties
- resolveContent: strips resolution metadata keys, merges overrides onto content

Bug fixes:
- resolution.ts: wrong type names (memberInvitation/memberApplication), no transaction,
  canceled as resolve outcome — all fixed; now uses db.txn() and delegates to handler
- inquiryCreate.ts: targetModel never set, @ts-nocheck — fixed
- inquiryUpdate.ts: @ts-nocheck removed
- inquiryCancel.ts: resolved guard updated for approved/denied statuses
- inquiryResolve.ts: canceled removed from outcomes, changesRequested added to resolvable statuses
- adminInquiryReadMany route: nonexistent searchableFields removed

New endpoint: POST /inquiries/:id/request-changes (target only)

Factory: inquiryFactory with sourceOrganization + targetUser dependencies

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@vercel

vercel Bot commented Mar 3, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
template-admin Error Error Mar 5, 2026 6:12am
template-superadmin Error Error Mar 5, 2026 6:12am
template-web Error Error Mar 5, 2026 6:12am

Request Review

…ution output

- Replace all string literals with InquiryStatus/InquiryType/InquiryResourceModel/Role enums
- handleApprove now returns Promise<Record<string, unknown> | void> — output data
  (e.g. spaceId from createSpace) gets merged into resolution automatically
- createSpace: spaceId belongs in resolution (output of approval), not content
- TERMINAL_STATUSES constant in assertUniqueInquiry for clarity

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…olve rules

InquiryAction: send, resolve, requestChanges, cancel, update, read

inquiry rebac rules:
- send: org admin+ for inviteOrganizationUser, org member+ for createSpace,
        space admin+ for updateSpace/transferSpace (uses type field rule)
- resolve: self (targetUserId) for user-targeted; superadmin bypass for admin-targeted
- requestChanges: delegates to resolve
- cancel/update: source-side mirror of send
- read: any source or target participant

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add assertInquiryPermission utility (hydrate + rebac check)
- Wire all inquiry controllers to use rebac actions (read, send, update,
  cancel, resolve, requestChanges)
- inquiryCreate: replace manual membership DB check with rebac send check
  on partial record; generalize source field assignment for all 4 types
- inquiryRead: use getResource + validatePermission middleware instead of
  manual fetch + access check
- inquirySent/inquiryReceived: replace getUserOrganizationIds DB query
  with c.get('organizationUsers') context (already loaded in request)
- Delete access.ts (fully replaced by rebac)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add validatePermission(action) to each route's middleware array so
resourceContextMiddleware + rebac check runs before the controller.
Controllers now use getResource<'inquiry'>(c) — no manual fetch or
permission check inline.

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

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- assertInquiryMutable + TERMINAL_STATUSES → validations/assertInquiryMutable.ts
- assertUniqueInquiry → validations/assertUniqueInquiry.ts (imports TERMINAL_STATUSES)
- Delete services/utils/assertUniqueInquiry.ts
- inquiryCancel: replace inline terminal status check with assertInquiryMutable

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

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

- Remove redundant validatePermission middleware from org/space create inquiry routes
- Remove organizationId from inviteOrganizationUser content schema (resolved from context)
- Split resolutionSchema into resolutionInputSchema (submittable) + resolutionSchema (full stored shape)
- Export baseResolutionInputSchema/BaseResolution from schemas; default InquiryHandler generics to it
- Make InquiryHandler generic <TContent, TResolution, TResolutionInput> with BaseResolution defaults
- handleApprove now receives typed TContent instead of Record<string, unknown>
- resolution.ts parses content through handler.contentSchema instead of casting
- resolveContent uses resolutionInputSchema to determine allowed override keys
- createSpace: add slug to content, remove redundant organizationId, export spaceContentSchema/SpaceContent
- createSpace: validate checks existing space + open inquiry by slug (Promise.all)
- updateSpace: content is Partial<SpaceContent> (name/slug optional), validate checks slug collision excluding self
- transferSpace: content is empty object (source/target are FKs from context)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
agreenspan and others added 7 commits March 4, 2026 21:43
…ssignability

Replaces property function type with method shorthand syntax so InquiryHandler<TContent>
is bivariant and assignable to InquiryHandler without casts or bivarianceHack.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…, camelCase constants

- InquiryHandler.validate now receives typed TContent as 3rd arg (no more inquiry.content casts)
- validateInquiryPreCreate service centralises unique check + handler.validate call for all 4 create controllers
- Deleted validateInquiryMutable.ts; moved inquiryTerminalStatuses into validateInquiryStatus.ts
- Renamed assert* → validate* for status guard functions (validateInquiryIsEditable, etc.)
- inquirySend controller now uses validateInquiryIsDraft instead of inline check
- RESOLUTION_METADATA_KEYS simplified to just explanation
- All CAPS_CASE constants renamed to camelCase (inquirySearchableFields, inquiryCreateSanitizeKeys, cacheReference)
- getValidatedBody/getValidatedQuery utils replace repeated c.req cast pattern
- INQUIRIES.md and CONTEXT.md updated to reflect all new patterns

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Pre-create calls synthesize only source+target fields, not a full DB record.
Partial<Inquiry> is honest about what's available; real Inquiry from update
is still assignable, so no call sites break. Removes the as Inquiry cast
in validateInquiryPreCreate.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Consistent with resolveInquiry which already uses db.txn. Ensures
future hooks (audit logs, notifications) fire atomically with the
status change.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Single-update operations are already atomic — no transaction needed.
Only resolve requires txn (handleApprove + inquiry.update).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@agreenspan

Copy link
Copy Markdown
Contributor Author

Code review

Found 3 issues:

  1. transferSpace target resolution always 404s — the handler declares targets: [{ targetModel: Organization, targetOrganizationId: 'targetOrganizationId' }] but its contentSchema is z.object({}). resolveInquiryTarget reads body.content['targetOrganizationId'], gets undefined, and throws 404 "Target organization not found" on every create request.

export const transferSpaceHandler: InquiryHandler<TransferSpaceContent> = {
sources: [{ sourceModel: InquiryResourceModel.Space, sourceSpaceId: 'spaceId' }],
targets: [{ targetModel: InquiryResourceModel.Organization, targetOrganizationId: 'targetOrganizationId' }],
contentSchema: z.object({}),
resolutionInputSchema: baseResolutionInputSchema,
resolutionSchema: baseResolutionInputSchema,
handleApprove: async (_db, _inquiry, _resolvedContent) => {

  1. Role elevation via update endpoint — a user with only manage org permission can create an inviteOrganizationUser inquiry with role: 'member', then PATCH the content to role: 'owner'. The permission re-check on update evaluates the existing inquiry (role=member, passes manage), not the incoming content, so the elevated role is persisted and takes effect on approval.

validateInquiryIsEditable(inquiry);
const handler = inquiryHandlers[inquiry.type];
const effectiveContent = handler.contentSchema.parse(content ?? inquiry.content);
if (handler.validate) await handler.validate(db, inquiry, effectiveContent);

  1. Stale target FK fields written to DB on create — all 4 create controllers spread ...body before ...target. resolveInquiryTarget returns only the matched FK (e.g. { targetModel: User, targetUserId }) without nulling out the others. A client that sends targetOrganizationId alongside targetModel: User will have that stale FK persisted, since inquiryCreateBodySchema passes all three optional FK fields through.

const inquiry = await db.inquiry.create({
data: { ...body, content: content as Prisma.InputJsonValue, ...source, ...target, sentAt: body.status === InquiryStatus.sent ? new Date() : null },
include: { targetUser: true, targetOrganization: true, targetSpace: true },
});
return respond.created(inquiry);
});

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

agreenspan and others added 5 commits March 5, 2026 00:15
…stale FK fields

- Collapse InquirySourceMeta/InquiryTargetMeta to { sourceModel/targetModel: InquiryResourceModel } — only field read at runtime
- Remove dead string content-key fields from all handler sources/targets declarations
- Fix resolveInquiryTarget to read body.targetOrganizationId/targetSpaceId directly (not via content key indirection) — fixes transferSpace always 404ing
- Bake null resets into resolveInquirySource/resolveInquiryTarget returns via nullSourceFields/nullTargetFields spread — prevents stale body FK fields leaking into DB on create
- Cast body target IDs to branded types (OrganizationId, SpaceId, UserId) at point of use

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Prevents role elevation via PATCH — content.role is evaluated by the
rebac send rule, so updating content without re-checking allowed
escalating to owner/admin without own permission.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Fix transferSpace handler: source-only uniqueness (one open transfer
  per space regardless of target org) via custom validate instead of
  validateUniqueInquiry which scoped by target too
- Fix updateSpace test: use fresh space for unique-slug test to avoid
  open inquiry from previous test blocking the unique check
- Fix updateSpace/createSpace handler tests: correct factory pattern
  (createSpace second-arg relations, ouCtx for createSpaceUser)
- Fix spaceInquiries POST tests: upgrade to owner role (updateSpace
  requires sourceSpace.own, transferSpace requires sourceSpace.org.own)
- Fix organizationInquiries createSpace tests: add required slug to content
- Fix inquiryUpdate role-elevation test: use admin user (not owner)
  since owner has own permission and can legitimately invite admin role
- Add target-can-read and unrelated-user-forbidden tests to inquiryRead

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…eld, clean up tests and docs

- Implement handleApprove for createSpace, updateSpace, transferSpace handlers
- Change unique: boolean → 'targeted' | 'untargeted' for explicit uniqueness semantics
- Fix Space.organizationId immutable field override so transferSpace can update org
- Trim InquiryAction to read/send/resolve (remove unused actions)
- Add superadmin bypass in createTestApp (platformRole=superadmin sets permix.setSuperadmin)
- Add handler approve tests: createSpace, updateSpace, transferSpace
- Fix --concurrency=1 → --max-concurrency=1 across all DB-backed packages
- Switch root test script from --filter to --cwd to eliminate log spam
- Fix navigation slice null guards on navigatePreservingContext/Spoof/All
- Fix ui.test.ts appName expectation to match slice default
- Update INQUIRIES.md, HOOKS.md, TESTING.md docs

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…, resolve permissions

- resolveInquiryTarget: remove handler param, use body.targetModel directly; add org slug + space slug alt lookups
- inquiryCreateBodySchema: add targetOrganizationSlug, targetSpaceSlug
- inviteOrganizationUser: consolidate 4 files into single index.ts
- Move validateInquiryStatus → validations/, resolveContent → services/ (delete utils/)
- ReBAC resolve: transferSpace now requires target org own (not manage)
- Update all callers to drop handler arg from resolveInquiryTarget
- Ticket and docs updated to reflect completed state

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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