fix(spectrum): report exhausted shared-line capacity - #97
Closed
Henry Zhang (caezium) wants to merge 1 commit into
Closed
Conversation
📝 WalkthroughWalkthroughProject creation now supports typed warnings and platformless projects. Platform toggles report iMessage connection warnings. Spectrum user enrollment reports structured failures in human-readable and JSON output. Contract tests and mock-server fixtures cover these responses. ChangesCLI outcome handling
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant API
participant Output
CLI->>API: Create project or toggle platform
API-->>CLI: Typed result with optional warning
CLI->>Output: Write JSON response or human-readable warning
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR updates the CLI’s Spectrum-related flows to correctly surface exhausted shared iMessage line capacity and missing phone connection scenarios, while also changing projects create so omitting --platforms creates a platformless project instead of inferring iMessage.
Changes:
- Adjusts
projects createto send an empty platform list when--platformsis omitted, and adds normalized non-blocking warnings for iMessage enrollment issues (including--jsonsupport). - Adds iMessage-specific warning handling to
spectrum platforms enable imessage, preserving the prior JSON shape when no warning is present. - Improves
spectrum users addfailure handling to emit structured non-zero JSON errors (and clearer human output), backed by expanded contract tests and mock-server behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/helpers/mock-server.ts | Expands mock server state and routes to simulate project create warnings, platform toggle warnings, deletes, and Spectrum user add failures. |
| tests/contract/projects.contract.test.ts | Adds contract coverage for platformless creation, iMessage enable warnings, and structured Spectrum user add failures. |
| tests/_setup.ts | Forces CI=1 during tests to prevent interactive prompts from hanging test runs. |
| src/lib/types.ts | Introduces local runtime result/warning shapes for project creation, platform toggles, and Spectrum user add failures. |
| src/commands/spectrum/users.ts | Parses structured API failures and emits JSON { error: { code, message } } with exit code 1 when requested. |
| src/commands/spectrum/platforms.ts | Reads optional warning responses when enabling iMessage and prints/returns normalized warning content only when relevant. |
| src/commands/projects.ts | Changes default behavior for omitted --platforms and adds normalized warning mapping/printing for iMessage-related create flows. |
| README.md | Updates command tree to reflect the projects create --platforms interface and “platformless when omitted” behavior. |
Suppressed comments (1)
src/commands/projects.ts:70
value in PROJECT_CREATE_WARNINGSalso matches inherited keys (e.g. "constructor"), which can cause untrusted warning codes to pass validation. Use an own-property check instead.
function isProjectCreateWarningCode(
value: unknown
): value is ProjectCreateWarningCode {
return (
typeof value === "string" && value in PROJECT_CREATE_WARNINGS
);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+60
to
+62
| function isOwnerWarningStatus(value: unknown): value is OwnerWarningStatus { | ||
| return typeof value === "string" && value in OWNER_STATUS_WARNING_CODES; | ||
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/helpers/mock-server.ts (1)
434-449: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest the explicit project warning independently.
The mock always emits
ownerStatuswithwarning. The owner-status fallback can therefore satisfy these tests ifreadProjectCreateWarningstops readingresult.warning.
tests/helpers/mock-server.ts#L434-L449: Add independent warning state that can return a recognized warning withoutownerStatus.tests/contract/projects.contract.test.ts#L156-L220: Add terminal and JSON cases for the warning-only response and assert the normalized local message.🤖 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 `@tests/helpers/mock-server.ts` around lines 434 - 449, Add independent warning state in tests/helpers/mock-server.ts around the project creation response so it can return a recognized warning without emitting ownerStatus; preserve existing owner-status behavior for other cases. In tests/contract/projects.contract.test.ts, add terminal and JSON scenarios covering the warning-only response and assert that readProjectCreateWarning produces the normalized local message.
🤖 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/projects.ts`:
- Around line 60-69: Update isOwnerWarningStatus and isProjectCreateWarningCode
to validate warning-code membership using an own-property check on
OWNER_STATUS_WARNING_CODES and PROJECT_CREATE_WARNINGS rather than the in
operator, so inherited keys such as constructor and __proto__ are rejected.
---
Nitpick comments:
In `@tests/helpers/mock-server.ts`:
- Around line 434-449: Add independent warning state in
tests/helpers/mock-server.ts around the project creation response so it can
return a recognized warning without emitting ownerStatus; preserve existing
owner-status behavior for other cases. In
tests/contract/projects.contract.test.ts, add terminal and JSON scenarios
covering the warning-only response and assert that readProjectCreateWarning
produces the normalized local message.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9762052e-5535-4130-8331-80d9809d513e
📒 Files selected for processing (8)
README.mdsrc/commands/projects.tssrc/commands/spectrum/platforms.tssrc/commands/spectrum/users.tssrc/lib/types.tstests/_setup.tstests/contract/projects.contract.test.tstests/helpers/mock-server.ts
Comment on lines
+60
to
+69
| function isOwnerWarningStatus(value: unknown): value is OwnerWarningStatus { | ||
| return typeof value === "string" && value in OWNER_STATUS_WARNING_CODES; | ||
| } | ||
|
|
||
| function isProjectCreateWarningCode( | ||
| value: unknown | ||
| ): value is ProjectCreateWarningCode { | ||
| return ( | ||
| typeof value === "string" && value in PROJECT_CREATE_WARNINGS | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use own-property checks for warning codes.
in accepts inherited keys such as "constructor" and "__proto__". A malformed API warning can pass isProjectCreateWarningCode, and readProjectCreateWarning can return a prototype member instead of a ProjectCreateWarning. Use an own-property check in both predicates.
Proposed fix
function isOwnerWarningStatus(value: unknown): value is OwnerWarningStatus {
- return typeof value === "string" && value in OWNER_STATUS_WARNING_CODES;
+ return (
+ typeof value === "string" &&
+ Object.prototype.hasOwnProperty.call(OWNER_STATUS_WARNING_CODES, value)
+ );
}
function isProjectCreateWarningCode(
value: unknown
): value is ProjectCreateWarningCode {
- return typeof value === "string" && value in PROJECT_CREATE_WARNINGS;
+ return (
+ typeof value === "string" &&
+ Object.prototype.hasOwnProperty.call(PROJECT_CREATE_WARNINGS, value)
+ );
}📝 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.
Suggested change
| function isOwnerWarningStatus(value: unknown): value is OwnerWarningStatus { | |
| return typeof value === "string" && value in OWNER_STATUS_WARNING_CODES; | |
| } | |
| function isProjectCreateWarningCode( | |
| value: unknown | |
| ): value is ProjectCreateWarningCode { | |
| return ( | |
| typeof value === "string" && value in PROJECT_CREATE_WARNINGS | |
| ); | |
| function isOwnerWarningStatus(value: unknown): value is OwnerWarningStatus { | |
| return ( | |
| typeof value === "string" && | |
| Object.prototype.hasOwnProperty.call(OWNER_STATUS_WARNING_CODES, value) | |
| ); | |
| } | |
| function isProjectCreateWarningCode( | |
| value: unknown | |
| ): value is ProjectCreateWarningCode { | |
| return ( | |
| typeof value === "string" && | |
| Object.prototype.hasOwnProperty.call(PROJECT_CREATE_WARNINGS, value) | |
| ); | |
| } |
🤖 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/projects.ts` around lines 60 - 69, Update isOwnerWarningStatus
and isProjectCreateWarningCode to validate warning-code membership using an
own-property check on OWNER_STATUS_WARNING_CODES and PROJECT_CREATE_WARNINGS
rather than the in operator, so inherited keys such as constructor and __proto__
are rejected.
Member
Author
|
Replaced by #98 after splitting the Dashboard and CLI work into separate Linear issues. The replacement is linked to ENG-2164 and removes the incorrect claim that this change introduced platformless creation. |
6 tasks
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
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.
Summary
--platformsis omitted, in both non-interactive and interactive usespectrum platforms enable imessagewhen no phone is connectedspectrum users addvisibly, with a structured non-zero JSON error when shared capacity is exhaustedBehavior
projects createno longer infers iMessage from an omitted--platformsflag. Human output explains that no platform was enabled and shows the follow-up enable command.When iMessage is explicit and owner enrollment is exhausted, the command returns success for the new project and writes the warning to stderr;
--jsonincludes the warning in the successful result. Other platform enables and iMessage disablement do not show the warning.The recovery copy offers another phone or a dedicated line. It does not suggest deleting a project or contacting support. Regression coverage keeps the warning after project deletion because deletion does not currently restore phone-wide shared-line capacity.
Upstream version
@photon-ai/dashboard-api@1.6.12— unchangedRoutes added/removed/changed
POST /api/projectsPOST /api/projects/:id/platforms/togglePOST /api/projects/:id/spectrum/usersSnapshot changes
New runtime dependencies
Testing
bun run build— passedCheckworkflow — passedThe repository root also contains unrelated untracked nested CLI worktrees, so bare local
bun testandbun run typecheckdiscover those directories. The scoped tracked-workspace commands above pass, and the clean PR checkout passes the complete check workflow; those untracked directories are not part of this PR.Checklist
@photon-ai/dashboard-apiversion bump, not hand edits (no API type changes)bun run checkpasses in the clean PR checkoutDashboard/API companion: photon-hq/dashboard#279
Linear: ENG-2161