feat: P2 batch — doctor, links, clone (#3, #7, #23)#43
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a new Changes
Sequence DiagramssequenceDiagram
actor User
participant CLI
participant Config
participant TokenService
participant API
User->>CLI: partiful doctor
CLI->>Config: Load credentials file
Config-->>CLI: config JSON / error
rect rgba(100,150,100,0.5)
Note over CLI,Config: Validate JSON and required fields
CLI->>CLI: Check apiKey, refreshToken, userId
end
rect rgba(100,100,150,0.5)
Note over CLI,TokenService: Refresh access token
CLI->>TokenService: Exchange refreshToken
TokenService-->>CLI: new token or error
end
rect rgba(150,100,100,0.5)
Note over CLI,API: Connectivity and basic API call
CLI->>API: POST /getMyUpcomingEventsForHomePage
API-->>CLI: response or error
end
CLI->>CLI: Collect env/platform info
CLI-->>User: Emit JSON envelope to stdout (and table to stderr when not json)
sequenceDiagram
actor User
participant CLI
participant EventsAPI
participant Storage
participant ImageService
User->>CLI: partiful events clone <eventId> --date "2026-04-22T19:00"
rect rgba(100,150,100,0.5)
Note over CLI,EventsAPI: Fetch source event
CLI->>EventsAPI: POST /getEvent { eventId }
EventsAPI-->>CLI: source event or error
end
rect rgba(150,150,100,0.5)
Note over CLI: Transform and merge overrides
CLI->>CLI: Parse new startDate, compute endDate (preserve duration unless overridden)
CLI->>CLI: Merge links, title, location, description, visibility, capacity, etc.
end
rect rgba(100,100,150,0.5)
Note over CLI,ImageService: Poster handling
alt poster is local file and not dry-run
CLI->>ImageService: upload image
ImageService-->>CLI: poster reference
else use existing poster ID/URL
CLI->>CLI: reuse poster
end
end
rect rgba(150,100,100,0.5)
Note over CLI,EventsAPI: Create cloned event
CLI->>EventsAPI: POST /createEvent (or print payload in dry-run)
EventsAPI-->>CLI: created event or error
end
CLI-->>User: Show payload/result (and exit code)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 4
🧹 Nitpick comments (1)
tests/doctor.test.js (1)
11-15: LetrunCli()callers override the credentials fixture.Because
PARTIFUL_CREDENTIALS_FILEis assigned after...env, every test is forced into the missing-file path. Moving the default before...envmakes it possible to add invalid-JSON and happy-path doctor cases with the same helper.Small cleanup
env: { ...process.env, - ...env, PARTIFUL_CREDENTIALS_FILE: '/tmp/__nonexistent_partiful_auth__.json', + ...env, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/doctor.test.js` around lines 11 - 15, The env spread in the test helper currently forces PARTIFUL_CREDENTIALS_FILE to always be '/tmp/__nonexistent_partiful_auth__.json' because it is added after ...env; update the helper used by runCli() so the default PARTIFUL_CREDENTIALS_FILE is set before spreading ...env (i.e., place PARTIFUL_CREDENTIALS_FILE: '/tmp/__nonexistent_partiful_auth__.json' before ...env) allowing callers of runCli() to override PARTIFUL_CREDENTIALS_FILE in their env argument for invalid-JSON and happy-path tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/doctor.js`:
- Around line 40-44: The try/catch that parses raw JSON should not return early;
instead when JSON.parse(raw) fails push the invalid config result
(results.push({ name: 'config_file', passed: false, detail: 'Invalid JSON' }))
and set the config variable (or parsed) to null so subsequent checks can run and
report skipped/passed state; remove the immediate return in the catch block so
doctor continues to run environment/platform checks (refer to variables parsed,
raw, config, and results to locate the code to change).
In `@src/commands/events.js`:
- Around line 467-485: Add an explicit opt-in re-invite flag to the clone
command: add .option('--reinvite', 'Re-invite guests from source event')
alongside the existing options on the command('clone') block and then forward
that options.reinvite boolean into the clone handler/path that performs the API
call (the same code that currently reads other overrides) so the clone logic
(e.g., where you call the cloneEvent/createClone function or build the clone
payload) includes a reinvite field when true; leave default behavior unchanged
when options.reinvite is falsy.
- Around line 544-554: The clone logic is resetting many toggle flags to
create-time defaults instead of copying the source event's settings; replace the
hard-coded true values for showHostList, showGuestCount, showGuestList,
showActivityTimestamps, displayInviteButton, allowGuestPhotoUpload,
enableGuestReminders, rsvpsEnabled, allowGuestsToInviteMutuals and
rsvpButtonGlyphType with values derived from the source event (src.<flag>) —
preserving the existing visibility logic (opts.private ? 'private' :
(src.visibility || 'public')) so that opts can still override when intended;
ensure any opt-based overrides remain handled but otherwise inherit each toggle
from src to accurately duplicate the source event.
- Around line 501-516: The dry-run branch currently forces sourceEvent = null
which causes clone previews to lose copied fields; instead, still perform the
read-only fetch with apiRequest when globalOpts.dryRun is true so the preview
can show the real source event (preserving fields like duration). Update the
logic around sourceEvent/apiRequest so that the POST '/getEvent' call with
token, getPayload and globalOpts.verbose runs for dry-run previews (but keep any
non-mutating behavior), and only treat a missing sourceEvent as an error via
jsonError when not in dry-run; keep using src = sourceEvent || {} for downstream
code.
---
Nitpick comments:
In `@tests/doctor.test.js`:
- Around line 11-15: The env spread in the test helper currently forces
PARTIFUL_CREDENTIALS_FILE to always be '/tmp/__nonexistent_partiful_auth__.json'
because it is added after ...env; update the helper used by runCli() so the
default PARTIFUL_CREDENTIALS_FILE is set before spreading ...env (i.e., place
PARTIFUL_CREDENTIALS_FILE: '/tmp/__nonexistent_partiful_auth__.json' before
...env) allowing callers of runCli() to override PARTIFUL_CREDENTIALS_FILE in
their env argument for invalid-JSON and happy-path tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a3bf5dca-0c98-4bdc-b326-43b2aae9d3ab
📒 Files selected for processing (5)
src/cli.jssrc/commands/doctor.jssrc/commands/events.jstests/doctor.test.jstests/events-integration.test.js
src/commands/doctor.js
Outdated
Comment on lines
+40
to
+44
| try { | ||
| parsed = JSON.parse(raw); | ||
| } catch { | ||
| results.push({ name: 'config_file', passed: false, detail: 'Invalid JSON' }); | ||
| return results; |
There was a problem hiding this comment.
Invalid JSON short-circuits the rest of doctor.
Line 44 returns immediately after the config_file failure, so the dependent checks are never marked as skipped and the independent environment / platform checks disappear entirely. Keep config = null and let the later checks report their own skipped/passed state.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/doctor.js` around lines 40 - 44, The try/catch that parses raw
JSON should not return early; instead when JSON.parse(raw) fails push the
invalid config result (results.push({ name: 'config_file', passed: false,
detail: 'Invalid JSON' })) and set the config variable (or parsed) to null so
subsequent checks can run and report skipped/passed state; remove the immediate
return in the catch block so doctor continues to run environment/platform checks
(refer to variables parsed, raw, config, and results to locate the code to
change).
Comment on lines
+467
to
+485
| .command('clone') | ||
| .description('Clone an existing event with a new date') | ||
| .argument('<eventId>', 'Source event ID') | ||
| .requiredOption('--date <date>', 'New event date (required)') | ||
| .option('--end-date <endDate>', 'End date/time (overrides duration preservation)') | ||
| .option('--title <title>', 'Override title') | ||
| .option('--location <location>', 'Override location name') | ||
| .option('--address <address>', 'Override street address') | ||
| .option('--description <desc>', 'Override description') | ||
| .option('--capacity <n>', 'Override guest limit', parseInt) | ||
| .option('--private', 'Make event private') | ||
| .option('--timezone <tz>', 'Override timezone') | ||
| .option('--theme <theme>', 'Override color theme') | ||
| .option('--effect <effect>', 'Override visual effect') | ||
| .option('--poster <posterId>', 'Override with built-in poster ID') | ||
| .option('--poster-search <query>', 'Override with poster search') | ||
| .option('--image <path>', 'Override with custom image') | ||
| .option('--link <url...>', 'Override links (repeatable)') | ||
| .option('--link-text <text...>', 'Display text for links') |
There was a problem hiding this comment.
Clone still has no opt-in re-invite path.
Lines 467-485 only expose create-style overrides. Issue #23 also calls out guest re-invite control, and without a --reinvite-style flag every clone is forced to drop the guest list.
If helpful, I can sketch a minimal flag/payload shape that preserves the current default while enabling explicit re-invites.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/events.js` around lines 467 - 485, Add an explicit opt-in
re-invite flag to the clone command: add .option('--reinvite', 'Re-invite guests
from source event') alongside the existing options on the command('clone') block
and then forward that options.reinvite boolean into the clone handler/path that
performs the API call (the same code that currently reads other overrides) so
the clone logic (e.g., where you call the cloneEvent/createClone function or
build the clone payload) includes a reinvite field when true; leave default
behavior unchanged when options.reinvite is falsy.
…ch, inherit source toggles, add --reinvite flag
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/commands/doctor.js (1)
40-47:⚠️ Potential issue | 🟠 MajorGuard
parsedbefore required-field checks.If JSON parsing fails,
parsedis undefined and Line 46 throws while evaluatingparsed[f]. That produces secondary error noise and duplicateconfig_filefailures.🔧 Proposed fix
- let parsed; + let parsed = null; try { parsed = JSON.parse(raw); } catch { results.push({ name: 'config_file', passed: false, detail: 'Invalid JSON' }); } - const required = ['apiKey', 'refreshToken', 'userId']; - const missing = required.filter(f => !parsed[f]); - if (missing.length > 0) { - results.push({ name: 'config_file', passed: false, detail: `Missing fields: ${missing.join(', ')}` }); - } else { - config = parsed; - results.push({ name: 'config_file', passed: true, detail: displayPath }); + if (parsed) { + const required = ['apiKey', 'refreshToken', 'userId']; + const missing = required.filter(f => !parsed[f]); + if (missing.length > 0) { + results.push({ name: 'config_file', passed: false, detail: `Missing fields: ${missing.join(', ')}` }); + } else { + config = parsed; + results.push({ name: 'config_file', passed: true, detail: displayPath }); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/doctor.js` around lines 40 - 47, The JSON parse error handler can leave parsed undefined, causing the required-field check (the required array and the missing = required.filter(f => !parsed[f]) logic) to throw; update the flow in the function that builds results so that after the catch you either skip the required-field checks or short-circuit (e.g., return or continue) when parsed is falsy, ensuring you only compute missing when parsed is an object, and keep pushing the single config_file failure into results rather than letting a secondary exception occur.
🧹 Nitpick comments (2)
src/commands/events.js (1)
521-522: Timezone parameter has no effect on date parsing.The
tzvalue is passed toparseDateTime(), but according to the implementation insrc/lib/dates.js, thetimezoneparameter is accepted but never used — all parsing operates in local/system timezone. Users may expect--timezoneto affect how--dateis interpreted, but it only sets the storedtimezonefield on the event.This is a pre-existing limitation, not introduced by this PR, but worth documenting or addressing separately.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/events.js` around lines 521 - 522, The tz argument passed from src/commands/events.js (tz computed and then passed to parseDateTime) is ignored because parseDateTime in src/lib/dates.js accepts but doesn't use the timezone parameter; update parseDateTime to apply the timezone parameter when parsing (e.g., use a timezone-aware parser like luxon/DateTime.fromISO / fromFormat with the zone option or moment.tz) so that parseDateTime(dateString, timezone) interprets the input in the provided zone, and ensure tests for parseDateTime and any callers (including the events command) validate that passing tz changes the resulting Date/time and stored timezone field accordingly.tests/doctor.test.js (1)
37-47: Add an invalid-JSON config regression test.The suite validates missing-file behavior, but it doesn’t cover a malformed credentials file (
doctorrequirement explicitly includes invalid JSON). Adding this case would protect the config parsing branch and skip-chain behavior.✅ Suggested test case
+ it('invalid config JSON marks config_file failed and continues envelope output', () => { + const badPath = '/tmp/__partiful_bad_auth__.json'; + require('fs').writeFileSync(badPath, '{not-json', 'utf8'); + const { stdout, exitCode } = runCli(['doctor'], { PARTIFUL_CREDENTIALS_FILE: badPath }); + expect(exitCode).not.toBe(0); + const parsed = JSON.parse(stdout.trim()); + expect(parsed.status).toBe('success'); + const configCheck = parsed.data.checks.find(c => c.name === 'config_file'); + expect(configCheck?.passed).toBe(false); + });Also applies to: 49-61
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/doctor.test.js` around lines 37 - 47, Add a new test in tests/doctor.test.js (alongside the existing "missing config file produces failed config_file check") that writes an intentionally malformed JSON credentials/config file at the same path the CLI expects, invokes runCli(['doctor']), then asserts the CLI returns a non-zero exitCode, the parsed JSON output has status 'success', contains a 'config_file' check (find via parsed.data.checks.find(c => c.name === 'config_file')), that this check.passed is false, and parsed.data.allPassed is false; ensure the test cleans up the malformed file after running so it doesn't affect other tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/doctor.js`:
- Around line 95-101: The unguarded read/parse of package.json using pkgPath and
pkg can throw and abort the whole doctor run; wrap the package.json read +
JSON.parse in a try/catch around the logic that builds the environment result
(the block that computes pkg and calls results.push with name: 'environment'),
and on error push an environment check with passed: false (or passed: true if
you prefer non-fatal) and include the caught error.message in detail so the
command records the failure instead of throwing; ensure normal success still
pushes CLI version and Node version when parse succeeds.
In `@src/commands/events.js`:
- Line 486: The --reinvite <statuses> option is declared but unused; update the
clone handler in src/commands/events.js to check opts.reinvite, parse the CSV
string into a Set of uppercased statuses, and after fetching the event's guests
filter guests whose status is in that set and call the same
invitation/re-invitation code path used elsewhere in this file (use the existing
invite/send-invite function used for initial invites) for each matched guest; if
you prefer not to implement, remove the .option('--reinvite <statuses>')
declaration to avoid a no-op flag.
- Around line 611-618: The update command is incorrectly accessing a
non-existent .poster property on the searchPosters result; change the block that
handles opts.posterSearch so it uses the poster object directly (i.e., pass
results[0] to buildPosterImage and assign the returned value to event.image)
rather than trying to read results[0].poster—update the code around
searchPosters, results[0], buildPosterImage and event.image to match the
create/clone command behavior.
---
Duplicate comments:
In `@src/commands/doctor.js`:
- Around line 40-47: The JSON parse error handler can leave parsed undefined,
causing the required-field check (the required array and the missing =
required.filter(f => !parsed[f]) logic) to throw; update the flow in the
function that builds results so that after the catch you either skip the
required-field checks or short-circuit (e.g., return or continue) when parsed is
falsy, ensuring you only compute missing when parsed is an object, and keep
pushing the single config_file failure into results rather than letting a
secondary exception occur.
---
Nitpick comments:
In `@src/commands/events.js`:
- Around line 521-522: The tz argument passed from src/commands/events.js (tz
computed and then passed to parseDateTime) is ignored because parseDateTime in
src/lib/dates.js accepts but doesn't use the timezone parameter; update
parseDateTime to apply the timezone parameter when parsing (e.g., use a
timezone-aware parser like luxon/DateTime.fromISO / fromFormat with the zone
option or moment.tz) so that parseDateTime(dateString, timezone) interprets the
input in the provided zone, and ensure tests for parseDateTime and any callers
(including the events command) validate that passing tz changes the resulting
Date/time and stored timezone field accordingly.
In `@tests/doctor.test.js`:
- Around line 37-47: Add a new test in tests/doctor.test.js (alongside the
existing "missing config file produces failed config_file check") that writes an
intentionally malformed JSON credentials/config file at the same path the CLI
expects, invokes runCli(['doctor']), then asserts the CLI returns a non-zero
exitCode, the parsed JSON output has status 'success', contains a 'config_file'
check (find via parsed.data.checks.find(c => c.name === 'config_file')), that
this check.passed is false, and parsed.data.allPassed is false; ensure the test
cleans up the malformed file after running so it doesn't affect other tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b18cc6eb-d463-4425-8f1d-1a8d0db0f38f
📒 Files selected for processing (3)
src/commands/doctor.jssrc/commands/events.jstests/doctor.test.js
…n update, remove dead --reinvite flag
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/commands/doctor.js (1)
40-52:⚠️ Potential issue | 🔴 CriticalInvalid JSON still crashes subsequent field checks.
When
JSON.parse(raw)throws (line 41),parsedremainsundefined. Line 46 then attemptsparsed[f], which throws aTypeError, short-circuiting the entirerunChecks()flow. The outercatchat line 54 will catch it, but the result detail will be misleading and independent checks (environment, platform) won't run properly.Guard against
undefinedbefore accessingparsed:🐛 Proposed fix
let parsed; try { parsed = JSON.parse(raw); } catch { results.push({ name: 'config_file', passed: false, detail: 'Invalid JSON' }); + parsed = null; } - const required = ['apiKey', 'refreshToken', 'userId']; - const missing = required.filter(f => !parsed[f]); - if (missing.length > 0) { - results.push({ name: 'config_file', passed: false, detail: `Missing fields: ${missing.join(', ')}` }); - } else { - config = parsed; - results.push({ name: 'config_file', passed: true, detail: displayPath }); + if (parsed) { + const required = ['apiKey', 'refreshToken', 'userId']; + const missing = required.filter(f => !parsed[f]); + if (missing.length > 0) { + results.push({ name: 'config_file', passed: false, detail: `Missing fields: ${missing.join(', ')}` }); + } else { + config = parsed; + results.push({ name: 'config_file', passed: true, detail: displayPath }); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/doctor.js` around lines 40 - 52, The JSON parse failure leaves parsed undefined and later code accesses parsed[f], causing a TypeError; guard the subsequent required-field checks by only running the required/missing logic when parsed is defined—wrap the const required / const missing / if (missing.length > 0) ... else ... block in an if (parsed) { ... } (or return/continue after pushing the 'Invalid JSON' result) so that when JSON.parse(raw) fails you skip field checks and avoid crashing runChecks(); refer to the parsed variable and the missing/required logic in src/commands/doctor.js.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/doctor.js`:
- Around line 21-25: The file defines an unused helper function mask(value,
visibleEnd = 4) that is dead code; either remove this function entirely or wire
it up where sensitive fields (e.g., apiKey, refreshToken) are printed. To fix,
delete the mask function definition from src/commands/doctor.js (and any related
unused imports) if you don't need masking, or apply mask(...) when formatting
any sensitive values before logging/output (search for places that would print
apiKey/refreshToken and call mask there) to ensure the helper is actually used.
---
Duplicate comments:
In `@src/commands/doctor.js`:
- Around line 40-52: The JSON parse failure leaves parsed undefined and later
code accesses parsed[f], causing a TypeError; guard the subsequent
required-field checks by only running the required/missing logic when parsed is
defined—wrap the const required / const missing / if (missing.length > 0) ...
else ... block in an if (parsed) { ... } (or return/continue after pushing the
'Invalid JSON' result) so that when JSON.parse(raw) fails you skip field checks
and avoid crashing runChecks(); refer to the parsed variable and the
missing/required logic in src/commands/doctor.js.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 83830573-defe-496e-863c-27a8ad0003d8
📒 Files selected for processing (2)
src/commands/doctor.jssrc/commands/events.js
🚧 Files skipped from review as they are similar to previous changes (1)
- src/commands/events.js
This was referenced Mar 26, 2026
This was referenced Mar 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3, closes #7, closes #23
What
Three P2 features implemented in parallel:
Doctor command (#3)
partiful doctor— 5 health checks (config, token, API, env, platform)--dry-run,--format json, exit code 1 on failureEvent links (#7)
--link <url>and--link-text <text>onevents createandevents updateClone events (#23)
partiful events clone <eventId> --date <date>--dry-runsupportTesting
Summary by CodeRabbit
New Features
doctorcommand for diagnostics (credentials, token/API checks, platform/runtime reporting, JSON or human-readable output, dry-run)--linkand--link-texttoevents create/events updatefor managing event linksevents cloneto duplicate events with overrides (title, date, timezone/theme/effect, visibility, location, capacity, image/poster)Tests
doctorcommand behavior and error casesevents clonedry-run behavior