Add a CLI command to edit your profile (bio, title, out-of-office) - #679
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
github-actions
Bot
added
commands
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 3 files
You’re at about 94% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
🟡 Changes recommended
Empty date flags can bypass mutual-exclusion validation, and related command documentation and error messaging need updates.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds CLI commands for updating the current user’s profile and managing out-of-office dates.
Changes:
- Adds profile update and out-of-office commands.
- Adds validation, SDK mutations, and profile readback.
- Adds tests and refreshes the CLI surface snapshot.
File summaries
| File | Description |
|---|---|
internal/commands/people.go |
Implements the commands and validation. |
internal/commands/people_test.go |
Tests profile and out-of-office workflows. |
.surface |
Records the expanded CLI surface. |
Review details
Suppressed comments (2)
internal/commands/people.go:362
dateparse.IsValidonly checks theYYYY-MM-DDshape (internal/dateparse/dateparse.go:149-152), so impossible values such as2026-99-99pass and are sent to the API. Validate the parsed values withtime.Parse("2006-01-02", ...), as incards.go:1125-1130andtodos.go:1586-1598, and move the range/date checks beforeensureAccountand the profile GET so invalid invocations reliably return local usage errors.
startDate := dateparse.Parse(start)
endDate := dateparse.Parse(end)
if !dateparse.IsValid(startDate) {
internal/commands/people.go:343
- The required-pair and date-format checks occur only after account resolution and
People().Me(). Thus--start 2026-09-14or an invalid date performs a GET first, and an authentication/network failure masks the deterministic usage error. Move all non-clear date validation ahead ofensureAccountand the profile lookup.
if err := ensureAccount(cmd, app); err != nil {
return err
}
me, err := app.Account().People().Me(cmd.Context())
- Files reviewed: 3/3 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d0e421eb63
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
🟡 Changes recommended
Multiple moderate command catalog, validation, guidance, and post-update error-handling issues remain unresolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (5)
Previously missed (1) — in code that hasn't changed since the last review.
internal/commands/people.go:280
- The PUT has already succeeded when this readback runs, so returning the GET error reports the command as failed even though the profile was mutated. Follow the existing project-update pattern: return success with a diagnostic and fallback payload when refreshing the latest state fails.
internal/commands/people.go:155
- These actions are registered in Cobra, but the
peopleentry inCommandCategories()still lists onlylist,show,pingable,add, andremove, sobasecamp commandsomits both new actions.STYLE.md:21-24requires registered commands to appear in the static catalog; add the canonicalupdateandout-of-officeactions there, and refresh the stale People entry inAPI-COVERAGE.mdas required byAGENTS.md:143-149.
cmd.AddCommand(newPeopleUpdateCmd())
cmd.AddCommand(newPeopleOutOfOfficeCmd())
internal/commands/people.go:175
- This helper is shared by the out-of-office command, so
people out-of-office 123currently reports that a profile cannot be edited and recommends the unrelatedpeople update ... --biocommand. Make the shared error command-neutral (or pass command-specific text) so the usage guidance matches both callers.
return output.ErrUsageHint(
"Only your own profile can be edited",
"The target is always \"me\": basecamp people update me --bio \"...\"")
internal/commands/people.go:197
- The help says an empty value clears any field, but
--first-week-day ""and--time-format ""are rejected by the enum validators below. Clarify that empty-value clearing applies to the free-text flags so the documented behavior matches the command.
Each field is left unchanged unless you pass its flag. Pass a flag with an
empty value to clear that field:
internal/commands/people.go:224
America/Chicagois an IANA identifier, not a Rails time-zone name. The API accepts both forms, so describe both rather than mislabeling the example.
cmd.Flags().StringVar(&timeZone, "time-zone", "", "Rails time zone name (e.g. America/Chicago)")
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Balanced
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
You’re at about 96% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 7 files (changes from recent commits).
You’re at about 96% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="internal/commands/people_test.go">
<violation number="1" location="internal/commands/people_test.go:1132">
P2: This test does not actually verify a failed read-back. Record the GET separately and assert it occurred, then assert the diagnostic through the configured output path so the fallback behavior is covered.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
🟡 Changes recommended
The missing out-of-office read path and incomplete preference-field responses must be addressed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
API-COVERAGE.md:212
- This row still reports all 12 People endpoints as implemented, but there are no CLI callers for the pinned SDK's
GetOutOfOffice,GetMyPreferences, orUpdateMyPreferencesmethods. Set/clear covers only two of the three out-of-office endpoints, and updating preference-shaped fields through/my/profile.jsondoes not cover the separate preference endpoints. Add command paths for these methods before retaining ✅, or revise the coverage accounting.
| people | 12 | `people`, `me` | ✅ | BC4 | - | list, show, update (edit your own profile via `PUT /my/profile.json`), out-of-office set/clear (`EnableOutOfOffice`/`DisableOutOfOffice`), pingable, add, remove (BC5: `tagline` alias of `bio` on person output) |
internal/commands/people.go:165
- This rationale overstates the endpoint restriction: the documented out-of-office endpoints accept any person ID, and Pro Pack admins may manage other people. The CLI can remain intentionally self-only, but describe that as a command-level scope so future work is not misled about the API capability.
// action. The profile PUT and the out-of-office endpoints only ever act on the
// authenticated user, so the target is always spelled "me" — matching
- Files reviewed: 8/8 changed files
- Comments generated: 4
- Review effort level: Balanced
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 02799b106d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
🟡 Changes recommended
Relative OOO dates need one shared reference time, and the PR description must match the implemented profile fields.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
internal/commands/people.go:165
- This endpoint rationale is inaccurate: the profile PUT is self-only, but the out-of-office API allows Pro Pack admins to manage another person. Keeping this CLI self-only is valid, but document it as a command policy so future admin-target support is not mistaken for an API limitation.
// requireMeTarget enforces the "me" sentinel that scopes a self-only person
// action. The profile PUT and the out-of-office endpoints only ever act on the
// authenticated user, so the target is always spelled "me" — matching
internal/commands/people.go:198
- The help now intentionally excludes
first_week_dayandtime_format, but the PR description still says this command exposes the endpoint’s full field set and validates both flags. Since the linked API/SDK contract includes those request fields, please update the PR description to reflect the narrowed scope (or restore the promised flags) so users and reviewers are not given conflicting behavior.
Week-start and time-format are account preferences, not profile fields, and
are not edited here.`,
- Files reviewed: 8/8 changed files
- Comments generated: 2
- Review effort level: Balanced
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62626911a5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Wire the shipped UpdateMyProfile SDK call to a new "basecamp people update me" command covering the full /my/profile.json field set (name, email, title, bio, location, time zone, first week day, time format), with pointer set/clear/omit flag semantics: a flag left off omits the field, a flag with an empty value clears it. Add "basecamp people out-of-office me" to set (--start/--end, natural language or YYYY-MM-DD) or clear (--clear) out-of-office via the person out-of-office endpoints. Both are scoped to the authenticated user with the "me" sentinel that "basecamp people show me" already accepts, leaving room for a future "people update <id>" admin edit.
Reject impossible calendar dates (e.g. 2026-13-45) locally with time.Parse rather than only regex-shape validation, and track --start and --end by flag presence so an explicit empty value is a malformed date instead of a silently-dropped omission — --clear --start "" now conflicts and --start "" reports the missing date. Add the missing --time-format enum tests to mirror --first-week-day.
…rage Register people update / out-of-office in the basecamp commands catalog, the People rows of API-COVERAGE.md, and the Basecamp skill so agents can discover the capability. Add smoke coverage: bare-invocation checks for both write commands and an out-of-scope declaration for the ooo alias. Reject an out-of-office range whose end precedes its start, and use a stable ordered example instead of the independently-resolved weekday pair. Keep a profile update reported as successful when the read-back GET fails, attaching a diagnostic rather than an error. Make the self-only target error command-neutral since update and out-of-office share it.
Week-start and time-format are account preferences read via GetMyPreferences and absent from the profile GET, so editing them through the profile PUT could not round-trip in the read-back and made the "clear with an empty value" contract untrue for those two enums. Drop them from people update; the command now covers name, email, title, bio, location, and time zone — all free text, all clearable. Make a no-flag "people out-of-office me" report current status via GetOutOfOffice so that read path is reachable from the CLI, and assert the read-back diagnostic and the JSON error/code fields in tests.
Both ends of a relative range now resolve from a single captured time.Now(), so an invocation that crosses local midnight cannot turn --start today --end today into a two-day range. Record the reachable GetOutOfOffice read in the API-COVERAGE People row.
There was a problem hiding this comment.
🔵 Needs a closer look
Correct the self-only endpoint claim and remove unsupported flags from the PR description.
Review details
Suppressed comments (2)
internal/commands/people.go:198
- This implementation explicitly excludes week-start and time-format, but the PR’s Commands section still promises
--first-week-dayand--time-format, calls this the endpoint’s full field set, and describes enum validation. Since their removal is intentional, update the PR description so users and release notes do not advertise unsupported flags.
Week-start and time-format are account preferences, not profile fields, and
are not edited here.`,
internal/commands/people.go:165
- This states that the out-of-office endpoints are self-only, but the embedded API contract says Pro Pack admins can manage other people (
internal/mcpserver/model/openapi.json:12472-12475and12649-12651). The CLI’s self-only scope is intentional, so describe it as a command restriction rather than endpoint behavior.
// action. The profile PUT and the out-of-office endpoints only ever act on the
// authenticated user, so the target is always spelled "me" — matching
- Files reviewed: 8/8 changed files
- Comments generated: 0 new
- Review effort level: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
The advertised preference flags conflict with the implementation, and People API coverage is overstated.
Review details
Suppressed comments (2)
API-COVERAGE.md:212
- This row still overstates People coverage. The 12 documented People operations include
GetMyPreferencesandUpdateMyPreferences, and the pinned SDK exposes those methods, but no CLI command calls either one; this PR now explicitly excludes--first-week-dayand--time-format. That leaves two in-scope SDK operations unreachable while the matrix claims ✅/12. Please add a separate preferences command with catalog/tests/surface/docs, consistent with the completeness requirement inAGENTS.md:131-149, or otherwise revise the coverage claim so it does not report all 12 as implemented.
| people | 12 | `people`, `me` | ✅ | BC4 | - | list, show, update (edit your own profile via `PUT /my/profile.json`), out-of-office show/set/clear (`GetOutOfOffice`/`EnableOutOfOffice`/`DisableOutOfOffice`), pingable, add, remove (BC5: `tagline` alias of `bio` on person output) |
internal/commands/people.go:198
- The PR description still promises
--first-week-dayand--time-formatas part of the command’s full field set, including local enum validation, while these lines explicitly exclude them and the added test expects the flags to be unknown. Since the omission is intentional, update the command and test descriptions in the PR metadata so the advertised CLI contract matches this implementation.
Week-start and time-format are account preferences, not profile fields, and
are not edited here.`,
- Files reviewed: 8/8 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Reword the me-target comment: scoping to yourself is a CLI restriction, not endpoint behavior — the out-of-office endpoints are person-scoped and an admin may manage others, while only the profile PUT is inherently self-only. Note in API-COVERAGE that GetMyPreferences/UpdateMyPreferences (week-start, time-format) are not covered and belong to a follow-up preferences command, so the People row does not imply full coverage.
|
Round addressed (a98606b) — Copilot's latest review-body points:
Preferences editing is deliberately out of this PR's scope (the card and the profile PUT are about bio/title/OOO); it wants its own command. CI is green. Ready for another look. |
There was a problem hiding this comment.
🟡 Changes recommended
API coverage status and totals must accurately reflect the deferred preference endpoints.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 1
- Review effort level: Balanced
The section is marked implemented at the section level (its ✅ and the 184-of-184 summary predate this additive PR and never depended on the /my/preferences.json endpoints — the row was ✅ covering only list/show/pingable/add/remove before this change). A per-operation "not covered" caveat contradicts that section-level accounting, so drop it; preferences editing remains a separate follow-up noted in the PR.
There was a problem hiding this comment.
🟢 Approval recommended
The only unresolved comment is a non-blocking method-ordering nit.
Review details
Suppressed comments (1)
internal/commands/people.go:321
STYLE.md:26-29requires each command constructor to be followed by itsRunE, with helpers afterward.resolveOOODatecurrently separatesnewPeopleOutOfOfficeCmdfromrunPeopleOutOfOffice; move this helper below the run function to preserve the repository's method ordering.
// resolveOOODate turns a natural-language or YYYY-MM-DD date into a calendar
// date the API accepts, rejecting both unparseable input and impossible dates
// (e.g. 2026-13-45) locally rather than sending them to the server. The caller
// passes one reference time so both ends of a range resolve against the same
// day even if the command runs across local midnight.
- Files reviewed: 8/8 changed files
- Comments generated: 0 new
- Review effort level: Balanced
|
Round addressed (e11a0bb). CI is fully green (22 checks; the earlier Go Security Checker red was an exogenous This round's one new point — the API-COVERAGE People row contradicting itself (✅ implemented, yet Notes claimed 2 of 12 uncovered): I reverted the caveat I'd added and explained the reasoning in that thread. In short: at origin/main the row was already ✅ / 12 covering only list/show/pingable/add/remove, so the section's ✅ and the 184-of-184 total never depended on the The earlier clusters are all resolved in pushed commits: impossible/inverted/empty-value date handling and single-reference-time resolution (out-of-office), read-back resilience with a diagnostic, catalog/surface/skill/smoke coverage, and — after reassessing — removing week-start/time-format from |
What & why
Agent Accessibility card 9663670734 ("Can't update/modify profile settings") had its API/SDK groundwork shipped — bc3#9925 (out-of-office), bc3-api#387 and bc3-api#391 (docs), and basecamp-sdk#211 (
UpdateMyProfile) — but no CLI command wired to any of it.basecamp people me/people show meonly display the profile; an agent had no way to edit a person's own bio, title, or out-of-office. This closes that gap.Commands
basecamp people update me— wired to the SDK'sUpdateMyProfile(PUT /my/profile.json). Fields, all free text:--name--email--title--bio--location--time-zoneFlag semantics mirror the SDK's pointer model: a flag left off omits the field, a flag with an empty value clears it (
--bio ""). The PUT returns no body, so the updated profile is read back and returned; if that read-back fails the update is still reported successful with a diagnostic.basecamp people out-of-office me(aliasooo) — with no flags it shows current status (GetOutOfOffice);--start/--endset it (EnableOutOfOffice), and--clearclears it (DisableOutOfOffice). Dates accept natural language ("next monday","in 2 weeks") orYYYY-MM-DD, both ends resolve against one reference time, and an end before the start is rejected.Design notes
mesentinel is a CLI restriction, not an endpoint one: the profile PUT is inherently self-only, while the out-of-office endpoints are person-scoped (a Pro Pack admin can manage others). The CLI exposes only the self case for now and reservespeople update <id>for a future admin edit.GetMyPreferences/UpdateMyPreferences), not profile fields — they are absent from the profile GET, so they do not belong on this command; a dedicatedpreferencescommand is a follow-up.API-COVERAGE.mdrecords them as not-yet-covered.Standard
--json/--agent/--quietoutput modes apply via the global envelope.Coverage in parallel surfaces
Both commands are registered in the
basecamp commandscatalog, the People rows ofAPI-COVERAGE.md, the Basecamp skill (skills/basecamp/SKILL.md), the CLI.surfacesnapshot, and smoke coverage (bare-invocation check forupdate, status read forout-of-office, OOS declaration for theoooalias).Tests
internal/commands/people_test.goasserts the exact request built for: set / clear (empty value present in body) / omit (unset flags absent), the bare-medefault, no-fields and non-merejection, read-back-failure resilience (diagnostic surfaced), and out-of-office show / enable / clear / conflict / missing-date / inverted-range / impossible-date / natural-language paths.Closes the CLI half of AA card 9663670734.