fix: complete cohost lifecycle and invite-link support - #75
Conversation
Replace raw cohost membership writes with Partiful request callables, add invite-link lifecycle commands, repair legacy stale membership, and route event create/update/clone through canonical invitations. Add schema coverage, unit/orchestration tests, skill documentation, and live-tested error handling for partial failures. Closes #74
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThe PR adds canonical cohost request and removal lifecycles, strict contact resolution, invite-link commands, event integration, typed APIs, Firestore reads, tests, and updated documentation. ChangesCohost lifecycle
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant EventCommand
participant CreateEventAPI
participant CohostLifecycleAPI
participant EventOutput
EventCommand->>CreateEventAPI: Create event with an empty cohost list
CreateEventAPI-->>EventCommand: Return event ID
EventCommand->>CohostLifecycleAPI: Send canonical cohost requests
CohostLifecycleAPI-->>EventCommand: Return invitation outcomes
EventCommand->>EventOutput: Report results and failures
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/commands/events.ts (1)
233-242: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract a shared helper for post-creation cohost invitation.
The pattern of extracting the new event ID (
typeof data === 'string' ? data : data?.id ?? result.result?.eventId, with theif (!id) throw ...guard) followed byinviteCohostBatch(id, cohostIds, [], makeCohostCall(...))is repeated at three sites: single create (Lines 281-295), series create (Lines 260-270), and clone (Lines 526-552). The dry-run preview array{ endpoint: '/createCohostRequest', params: { targetUserId: cohostId } }is also duplicated verbatim between create (Lines 236-239) and clone (Lines 527-530).Because these are separate, independently maintained code blocks, a future change to the endpoint, invite params, or ID-extraction fallback order can silently drift between sites (for example, the dry-run preview could get out of sync with the actual runtime call). Extract a shared function to keep the behavior and its preview in one place.
♻️ Proposed shared helper
async function createEventAndInviteCohosts( token: string, config: ReturnType<typeof loadConfig>, payload: Record<string, unknown>, cohostIds: string[], verbose?: boolean, ): Promise<{ id: string; inviteResults: Awaited<ReturnType<typeof inviteCohostBatch>> }> { const result = await apiRequest('POST', '/createEvent', token, payload, verbose) as { result?: { data?: string | { id?: string }; eventId?: string }; }; const data = result.result?.data; const id = typeof data === 'string' ? data : data?.id ?? result.result?.eventId; if (!id) throw new Error('Partiful did not return an event ID'); const inviteResults = await inviteCohostBatch(id, cohostIds, [], makeCohostCall(token, config, verbose)); return { id, inviteResults }; } function planCohostInvites(cohostIds: string[]): Array<{ endpoint: string; params: { targetUserId: string } }> { return cohostIds.map((cohostId) => ({ endpoint: '/createCohostRequest', params: { targetUserId: cohostId } })); }Also applies to: 260-270, 281-295, 526-552
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/commands/events.ts` around lines 233 - 242, Extract shared createEventAndInviteCohosts and planCohostInvites helpers in events.ts, preserving the existing event-ID fallback order and missing-ID error. Replace the repeated single-create, series-create, and clone runtime invitation blocks with the shared helper, and use planCohostInvites for both create and clone dry-run previews so endpoint and parameter construction remain consistent.src/commands/cohosts.ts (1)
154-157: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid a redundant Firestore read for
cohostIds.
getCohostState(154) already callsgetCohostRequestsandgetCohostIdsinternally and merges them. CallinggetCohostIdsagain on the next line (156) issues a third Firestore GET for data already fetched insidegetCohostState, purely to get raw IDs for the stale-repair closure.The
addcommand avoids this by callinggetCohostRequestsandgetCohostIdsdirectly and merging withmergeCohostStatelocally instead of going throughgetCohostState. Mirroring that pattern here removes one network round trip on everycohosts removeinvocation.♻️ Proposed refactor to avoid the duplicate read
- const [states, currentIds] = await Promise.all([ - getCohostState(eventId, token, verbose), - getCohostIds(eventId, token, verbose), - ]); + const [requests, currentIds] = await Promise.all([ + getCohostRequests(eventId, token, verbose), + getCohostIds(eventId, token, verbose), + ]); + const states = mergeCohostState(requests, currentIds);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/commands/cohosts.ts` around lines 154 - 157, Update the cohosts remove flow around getCohostState to avoid fetching cohost IDs twice: call getCohostRequests and getCohostIds directly, merge their results with mergeCohostState, and retain the raw IDs for the stale-repair closure. Preserve the existing remove behavior while eliminating the redundant getCohostState-triggered read.src/lib/api/endpoints.ts (1)
194-226: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider
z.looseObject()instead of.passthrough().These five new schemas use
z.object({}).passthrough(). Zod 4 still supports.passthrough(), but treats it as legacy: "These methods are still available for backwards compatibility, and they will not be removed." The library also recommends the newer top-level constructor for new schemas going forward.Since all five schemas are new code in this PR, switching to
z.looseObject({})(andz.looseObject({ path: z.string().optional() })for the link-generation schema) aligns with the current recommended API without changing behavior.♻️ Proposed refactor to the new schemas
-export const CreateCohostRequestResponseSchema = z.object({}).passthrough(); +export const CreateCohostRequestResponseSchema = z.looseObject({}); ... -export const DeleteCohostRequestResponseSchema = z.object({}).passthrough(); +export const DeleteCohostRequestResponseSchema = z.looseObject({}); ... -export const RemoveCohostResponseSchema = z.object({}).passthrough(); +export const RemoveCohostResponseSchema = z.looseObject({}); ... -export const GenerateEventCohostLinkResponseSchema = z.object({ path: z.string().optional() }).passthrough(); +export const GenerateEventCohostLinkResponseSchema = z.looseObject({ path: z.string().optional() }); ... -export const RevokeEventCohostLinkResponseSchema = z.object({}).passthrough(); +export const RevokeEventCohostLinkResponseSchema = z.looseObject({});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/api/endpoints.ts` around lines 194 - 226, Replace the legacy .passthrough() calls in CreateCohostRequestResponseSchema, DeleteCohostRequestResponseSchema, RemoveCohostResponseSchema, GenerateEventCohostLinkResponseSchema, and RevokeEventCohostLinkResponseSchema with z.looseObject(), preserving the existing fields including optional path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/commands/cohosts.ts`:
- Around line 116-136: Replace the manual invitation loop in the cohosts command
with the shared inviteCohostBatch function from ../lib/cohosts.js, removing the
direct inviteCohost import. Pass the existing event, IDs, state, call, and
currentIds stale-repair behavior through inviteCohostBatch, preserving its
standardized error formatting and the existing ApiError/jsonOutput handling.
In `@src/lib/cohosts.ts`:
- Around line 113-123: Prevent setCohostIds from blindly replacing a
concurrently changed cohostIds value during stale repair. Before the PATCH,
re-read the current cohostIds and compare it with the IDs supplied from the
earlier read; detect drift and avoid overwriting concurrent changes, or
explicitly enforce the documented single-writer assumption if that is the
established contract. Keep the repair scope limited to setCohostIds and preserve
its deduplication behavior.
---
Nitpick comments:
In `@src/commands/cohosts.ts`:
- Around line 154-157: Update the cohosts remove flow around getCohostState to
avoid fetching cohost IDs twice: call getCohostRequests and getCohostIds
directly, merge their results with mergeCohostState, and retain the raw IDs for
the stale-repair closure. Preserve the existing remove behavior while
eliminating the redundant getCohostState-triggered read.
In `@src/commands/events.ts`:
- Around line 233-242: Extract shared createEventAndInviteCohosts and
planCohostInvites helpers in events.ts, preserving the existing event-ID
fallback order and missing-ID error. Replace the repeated single-create,
series-create, and clone runtime invitation blocks with the shared helper, and
use planCohostInvites for both create and clone dry-run previews so endpoint and
parameter construction remain consistent.
In `@src/lib/api/endpoints.ts`:
- Around line 194-226: Replace the legacy .passthrough() calls in
CreateCohostRequestResponseSchema, DeleteCohostRequestResponseSchema,
RemoveCohostResponseSchema, GenerateEventCohostLinkResponseSchema, and
RevokeEventCohostLinkResponseSchema with z.looseObject(), preserving the
existing fields including optional path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7f29dfd8-6a7f-4aed-a609-ada88083c37a
📒 Files selected for processing (13)
docs/plans/2026-08-01-cohost-lifecycle-and-links.mdskills/partiful/SKILL.mdskills/partiful/references/events.mdskills/partiful/references/guests-invitations-and-cohosts.mdsrc/commands/cohosts.tssrc/commands/events.tssrc/commands/schema.tssrc/lib/api/endpoints.tssrc/lib/cohosts.tssrc/lib/http.tstests/cohosts.test.jstests/http.test.jstests/schema-api.test.js
| const call = callable(token, config, verbose); | ||
| const succeeded: Array<{ userId: string; outcome: string }> = []; | ||
| const failed: Array<{ userId: string; error: string }> = []; | ||
| for (const userId of ids) { | ||
| const state = states.find((item) => item.userId === userId); | ||
| const repairStale = state?.status === 'stale' | ||
| ? async () => { | ||
| currentIds = currentIds.filter((id) => id !== userId); | ||
| await setCohostIds(eventId, currentIds, token, verbose); | ||
| } | ||
| : undefined; | ||
| try { | ||
| succeeded.push(await inviteCohost(eventId, userId, state, call, repairStale)); | ||
| } catch (error) { | ||
| failed.push({ userId, error: String(error) }); | ||
| } | ||
| } | ||
| if (failed.length > 0) { | ||
| throw new ApiError('One or more co-host invitations failed', { eventId, succeeded, failed }); | ||
| } | ||
| jsonOutput({ eventId, results: succeeded, url: `https://partiful.com/e/${eventId}` }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Reuse inviteCohostBatch instead of reimplementing its loop.
This loop duplicates inviteCohostBatch from src/lib/cohosts.ts, with one difference: it reports failures as error: String(error) instead of error instanceof Error ? error.message : String(error). That means the error-message format tested for inviteCohostBatch at tests/cohosts.test.js (lines 127-136) is not the format that actually runs for the cohosts add command, and any future change to the batch-invite behavior in the library has to be duplicated here by hand.
inviteCohostBatch already accepts a per-user repairStale(userId) hook with the same "only call it when stale" guard this code implements manually, so the stale-ID-removal closure over currentIds can be passed straight through.
♻️ Proposed refactor to reuse inviteCohostBatch
- const call = callable(token, config, verbose);
- const succeeded: Array<{ userId: string; outcome: string }> = [];
- const failed: Array<{ userId: string; error: string }> = [];
- for (const userId of ids) {
- const state = states.find((item) => item.userId === userId);
- const repairStale = state?.status === 'stale'
- ? async () => {
- currentIds = currentIds.filter((id) => id !== userId);
- await setCohostIds(eventId, currentIds, token, verbose);
- }
- : undefined;
- try {
- succeeded.push(await inviteCohost(eventId, userId, state, call, repairStale));
- } catch (error) {
- failed.push({ userId, error: String(error) });
- }
- }
+ const call = callable(token, config, verbose);
+ const repairStale = async (userId: string) => {
+ currentIds = currentIds.filter((id) => id !== userId);
+ await setCohostIds(eventId, currentIds, token, verbose);
+ };
+ const { succeeded, failed } = await inviteCohostBatch(eventId, ids, states, call, repairStale);This requires importing inviteCohostBatch from ../lib/cohosts.js and no longer needs the direct inviteCohost import in this command.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const call = callable(token, config, verbose); | |
| const succeeded: Array<{ userId: string; outcome: string }> = []; | |
| const failed: Array<{ userId: string; error: string }> = []; | |
| for (const userId of ids) { | |
| const state = states.find((item) => item.userId === userId); | |
| const repairStale = state?.status === 'stale' | |
| ? async () => { | |
| currentIds = currentIds.filter((id) => id !== userId); | |
| await setCohostIds(eventId, currentIds, token, verbose); | |
| } | |
| : undefined; | |
| try { | |
| succeeded.push(await inviteCohost(eventId, userId, state, call, repairStale)); | |
| } catch (error) { | |
| failed.push({ userId, error: String(error) }); | |
| } | |
| } | |
| if (failed.length > 0) { | |
| throw new ApiError('One or more co-host invitations failed', { eventId, succeeded, failed }); | |
| } | |
| jsonOutput({ eventId, results: succeeded, url: `https://partiful.com/e/${eventId}` }); | |
| const call = callable(token, config, verbose); | |
| const repairStale = async (userId: string) => { | |
| currentIds = currentIds.filter((id) => id !== userId); | |
| await setCohostIds(eventId, currentIds, token, verbose); | |
| }; | |
| const { succeeded, failed } = await inviteCohostBatch(eventId, ids, states, call, repairStale); | |
| if (failed.length > 0) { | |
| throw new ApiError('One or more co-host invitations failed', { eventId, succeeded, failed }); | |
| } | |
| jsonOutput({ eventId, results: succeeded, url: `https://partiful.com/e/${eventId}` }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/commands/cohosts.ts` around lines 116 - 136, Replace the manual
invitation loop in the cohosts command with the shared inviteCohostBatch
function from ../lib/cohosts.js, removing the direct inviteCohost import. Pass
the existing event, IDs, state, call, and currentIds stale-repair behavior
through inviteCohostBatch, preserving its standardized error formatting and the
existing ApiError/jsonOutput handling.
| /** @deprecated Lifecycle commands must use canonical callables, not raw membership writes. */ | ||
| export async function setCohostIds( | ||
| eventId: string, | ||
| ids: string[], | ||
| token: string, | ||
| verbose = false, | ||
| ): Promise<void> { | ||
| const unique = [...new Set(ids.filter(Boolean))]; | ||
| const fields = { | ||
| cohostIds: { | ||
| arrayValue: { values: unique.map((id) => ({ stringValue: id })) }, | ||
| }, | ||
| }; | ||
| const fields = { cohostIds: { arrayValue: { values: unique.map((id) => ({ stringValue: id })) } } }; | ||
| await firestoreRequest('PATCH', eventId, { fields }, token, ['cohostIds'], verbose); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Blind overwrite of cohostIds during stale repair can lose concurrent updates.
setCohostIds writes the full cohostIds array back to Firestore based on a value read earlier (existingIds/currentIds in src/commands/cohosts.ts, used by both the add and remove command's stale-repair hooks). Nothing checks that cohostIds is still what was read before the PATCH is issued. If another writer changes cohostIds between the read and this write, that change is silently discarded because the PATCH replaces the whole field.
This function is documented as the narrow exception for repairing legacy corruption, so the blast radius is limited to that path, but a lost update here would silently drop a cohost ID that another actor just added or removed. Consider re-reading cohostIds immediately before the write and detecting drift, or confirming (and documenting) that this repair path is expected to run under a single-writer assumption.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/cohosts.ts` around lines 113 - 123, Prevent setCohostIds from blindly
replacing a concurrently changed cohostIds value during stale repair. Before the
PATCH, re-read the current cohostIds and compare it with the IDs supplied from
the earlier read; detect drift and avoid overwriting concurrent changes, or
explicitly enforce the documented single-writer assumption if that is the
established contract. Keep the repair scope limited to setCohostIds and preserve
its deduplication behavior.
1 participant
Summary
cohostIdswrites with Partiful's canonical cohost request lifecyclecohosts linkinspect/enable/disable workflow and schema/API discoverycohostIds-only corruption before issuing a canonical requestProduction API findings
{ eventId, targetUserId }INTERNAL; repair must first remove that stale IDevents/{eventId}/private/cohostSecretVerification
npm test(273 passed, 6 skipped)npm run typecheckgit diff --checkpending; repeat add was an idempotent no-opAPPROVE_WITH_NITS; fixed duplicate-read race and re-ran full suiteNotes
The initial live
INTERNALresponses were caused by two production contract details discovered during verification: the callable parameter istargetUserId, notcohostId, and legacy stale membership must be cleared before canonical repair. Both are covered by the implementation and tests.Closes #74
Summary by CodeRabbit
New Features
Bug Fixes
Documentation