AIT-376: set and verify the alert phone from the CLI - #52
Conversation
The alert phone could be set from the web dialog or through MCP, but not from the terminal — so a CLI-first user had to open a browser or drive an agent to opt into breakage alerts. `alerts phone status | set <phone> | verify <code>`, mirroring the MCP tools against the same user-scoped /auth/phone routes. Consents follow MCP rather than the web dialog: operational on, product and marketing behind --product / --marketing. A bare `set` must not silently consent someone to marketing. --json cannot prompt, so `set --json` reports the code is out and points at `verify`; the interactive path prompts inline. A delivery that did not report `sent` says so and stops instead of asking for a code that was never sent.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds CLI commands to view, register, verify, and remove an authenticated user’s alert phone. The commands validate inputs, manage consent and delivery preferences, support JSON output, and integrate with the root CLI. Tests cover status, validation, delivery, consent, verification, and removal behavior. ChangesAlert phone commands
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant AlertCommands
participant AuthPhoneAPI
CLI->>AlertCommands: run alerts phone set
AlertCommands->>AuthPhoneAPI: submit phone and preferences
AuthPhoneAPI-->>AlertCommands: return verification challenge
AlertCommands->>AuthPhoneAPI: submit six-digit code
AuthPhoneAPI-->>CLI: return verification result
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/alerts.ts`:
- Around line 61-76: Update the phone validation before the /auth/phone request
in the alerts command to require a complete E.164-shaped value: a leading plus
followed by 1–15 digits, rejecting “+”, non-digit suffixes, and overlong
numbers. Preserve the existing ValidationError behavior and add tests covering
malformed plus-prefixed inputs.
🪄 Autofix
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: aac4d592-a9bd-432d-a88c-54e62d860cd1
📒 Files selected for processing (3)
src/commands/__tests__/alerts.test.tssrc/commands/alerts.tssrc/index.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 13590300e5
ℹ️ 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".
- Validate a full E.164 shape (^\+[1-9]\d{1,14}$) instead of a bare leading
'+', so '+', '+abc', and over-length values are rejected before any
registration request (CodeRabbit).
- Refuse to start a verification when there is no TTY and neither --json nor
--code is given, before the code is sent — otherwise a CI/redirected caller
spends a code and quota, then blocks on a prompt that never reads (Codex P2).
Interactivity is injectable for tests.
|
@coderabbitai review |
|
@codex review |
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 29fbb30fbe
ℹ️ 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".
A malformed --code could only be rejected locally, but the send already went out first — burning delivery quota and superseding any live challenge.
|
@coderabbitai review |
|
@codex review |
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ac54c4c78
ℹ️ 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".
Drops the --product/--marketing opt-in flags (unreleased) and the consent mechanics from README and changelog. The set call now sends all three consents true; opt-out lives in the web app. 1078 tests green.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@README.md`:
- Line 242: Update the README sentence describing the 6-digit confirmation code
and `set` so it states that the prompt occurs only after successful delivery;
also explain that when delivery fails or no code arrives, `alertPhoneSet` exits
and users should retry, while preserving the existing `--json`, verification,
and `--code` guidance.
🪄 Autofix
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: 131aed40-02c8-4597-8b1b-844ce2004258
📒 Files selected for processing (4)
CHANGELOG.mdREADME.mdsrc/commands/__tests__/alerts.test.tssrc/commands/alerts.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/commands/tests/alerts.test.ts
- src/commands/alerts.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/commands/alerts.ts (1)
204-211: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd help examples for
remove.The new command has no
addExamples(removeCmd, ...)block. Add examples for interactive removal and the--yesand--jsonpaths so users can discover the confirmation behavior.Proposed addition
+ addExamples( + removeCmd, + ` +EXAMPLES: + $ hookmyapp alerts phone remove + $ hookmyapp alerts phone remove --yes + $ hookmyapp alerts phone remove --json +`, + );🤖 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/alerts.ts` around lines 204 - 211, Add an addExamples(removeCmd, ...) block alongside the phone remove command, covering interactive removal, non-interactive removal with --yes, and JSON output with --json. Follow the existing alert command example conventions and ensure the examples clearly expose the confirmation behavior.
🤖 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/alerts.ts`:
- Around line 174-176: Update the JSON-mode branch in the alert phone DELETE
flow to pass null to output when status is undefined, while preserving the
existing status output for non-empty responses.
---
Nitpick comments:
In `@src/commands/alerts.ts`:
- Around line 204-211: Add an addExamples(removeCmd, ...) block alongside the
phone remove command, covering interactive removal, non-interactive removal with
--yes, and JSON output with --json. Follow the existing alert command example
conventions and ensure the examples clearly expose the confirmation behavior.
🪄 Autofix
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: 46ae0fc8-3dc9-4b9b-948c-c1ed25cd9db2
📒 Files selected for processing (3)
README.mdsrc/commands/__tests__/alerts.test.tssrc/commands/alerts.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- README.md
- src/commands/tests/alerts.test.ts
A supplied code belongs to an already-sent challenge, so starting a new one superseded it and rejected the very code the caller was handed — --code now routes straight to verify with a single API call. DELETE prints null instead of undefined under --json on an empty response. README describes the failed-delivery path.
- Validate a full E.164 shape (^\+[1-9]\d{1,14}$) instead of a bare leading
'+', so '+', '+abc', and over-length values are rejected before any
registration request (CodeRabbit).
- Refuse to start a verification when there is no TTY and neither --json nor
--code is given, before the code is sent — otherwise a CI/redirected caller
spends a code and quota, then blocks on a prompt that never reads (Codex P2).
Interactivity is injectable for tests.
AIT-376: set and verify the alert phone from the CLI
The alert phone (AIT-370) could be set from the web launchpad dialog or through
MCP (
set_alert_phone/verify_alert_phone/get_alert_phone_status), butnot from the terminal. A CLI-first user had to open a browser or drive an agent
to opt into breakage alerts.
Commands
setflags:--sms(deliver by SMS instead of WhatsApp),--product,--marketing,--code <code>(skip the prompt, for scripts).All three hit the existing user-scoped
/auth/phoneroutes — no new API surface.Three decisions worth review
Consents follow MCP, not the web dialog. Operational is on; product and
marketing are opt-in behind flags. A bare
setmust not silently consentsomeone to marketing. Note this leaves the web dialog inconsistent — it
currently sends all three
trueafter its consent checkboxes were removed.That mismatch is tracked on AIT-366 and is not resolved here.
--jsoncannot prompt.set --jsonreturns{delivery:"sent", verified:false, next:"alerts phone verify <code>"}and thecaller finishes with
verify— the same two-step shape MCP uses. Codes arenever echoed into
--jsonoutput or logs.A failed delivery does not ask for a code. If
deliveryis notsent, thecommand says so and stops rather than prompting for something that was never
sent. This is the CLI-side counterpart of hookmyapp#196, which stopped the
backend reporting
sentfor messages the provider had rejected.Tests
6 new tests: national-format numbers rejected before any API call, consent
defaults, failed-delivery path making no verify call,
--codeskipping theprompt, and local code-shape validation. Full suite: 1075 pass.
Summary by CodeRabbit