Skip to content

feat(sip-trunk): show the environment's SIP address - #20

Merged
purpshell merged 8 commits into
devfrom
t3code/calls-sip-address
Sep 22, 2026
Merged

purpshell merged 8 commits into
devfrom
t3code/calls-sip-address

Conversation

@purpshell

@purpshell purpshell commented Sep 19, 2026 •

Copy link
Copy Markdown
Member

Summary

  • Adds polymorfa sip-trunk endpoint, which reads GET /platform/sip/endpoint and prints the SIP host, each transport with its port and whether SRTP is required, the RTP UDP port range, and one line of allow-list guidance. When the API returns status: "sip_not_hosted", it prints This environment has no SIP address. and exits 0 without printing an address. --json emits the standard {data, metadata} document, with data set to the raw SipEndpoint object. API errors use the existing mapping (for example, 403 exits with code 3).
  • sip-trunk create (when it returns inbound credentials) and sip-trunk rotate-credentials print a SIP address block after the one-time password in human output only. The optional lookup is a second request with a five-second upper bound, no retries, and any shorter global --timeout preserved. If it fails, the command keeps the password visible and exits 0. JSON and quiet modes make no extra request.
  • The response is validated before rendering. A sip_not_hosted response that carries a host, or a hosted response with no host or with invalid ports, fails as invalid_response, so the CLI never prints an invented address.
  • Updates the README SIP trunks section and docs/command-research/calls.md. The skills/ docs do not mention sip-trunk commands.

Example (hosted):

Host	sip.example.test
Transport	UDP 5060, SRTP not supported
Transport	TCP 5060, SRTP not supported
Transport	TLS 5061, SRTP required
RTP	UDP 20000-20999
Allow SIP and RTP in both directions between your PBX and the addresses sip.example.test resolves to.

Example (sip_not_hosted):

This environment has no SIP address.

Dependency on the SDK

The command calls client.sipTrunks.endpoint() from @polymorfa/sdk 0.1.0-dev.20260922115749 (published from polymorfa/sdks#277's merge, 8bc00778b). The CLI still checks the response shape locally (getSipEndpoint in src/services/sip-trunks.ts) so a malformed answer fails instead of printing a partial address. The API contract is getSipEndpoint on monorepo dev (merge of polymorfa/polymorfa#229, cdc7ec09a), scope sessions:read.

Verification

Current-head CI passes on 649c0392b091bf7e8448904c4770b1b6d5a10904, including the complete verification workflow. The 13 local SIP command tests include real child processes with an HTTP endpoint that never responds: credentials print first, the default appendix stops after five seconds, and a 500 ms global timeout is respected. The shorter-timeout regressions failed before the fix and pass after it.

Typecheck, lint and formatting pass. The tests use a local HTTP server; no deployed API or real PBX call was exercised.

Six-part assessment

  • API: unaffected runtime; consumes merged polymorfa/polymorfa#229 (GET /platform/sip/endpoint, sessions:read).
  • SDKs: updated. feat(sip-trunks): add Client.sipTrunks.endpoint for the environment's SIP address sdks#277 (merged, 8bc00778b) adds sipTrunks.endpoint(); this CLI pins @polymorfa/sdk@0.1.0-dev.20260922115749 and uses it.
  • CLI: updated (this PR).
  • Docs: CLI README and command research doc updated. Public docs change in the monorepo PR.
  • Feature releases: no flag or audience change. Unified Calls remains unreleased to customers. Environments without a SIP address report sip_not_hosted.
  • Admin controls: unaffected. The SIP address is deployment configuration with no per-team state.

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith with what you need. Autofix is disabled.

Add `sip-trunk endpoint`, which prints the SIP host, each transport with its
port and SRTP requirement, the RTP UDP range and allow-list guidance, and
reports sip_not_hosted without printing an address. `create` and
`rotate-credentials` print the address after new inbound credentials in
human output; a failed lookup never hides the password.

The pinned SDK lacks sipTrunks.endpoint, so the command reads
GET /platform/sip/endpoint through the SDK raw client and validates the
response. Switch to the SDK method once it is released and pinned.
@coderabbitai

coderabbitai Bot commented Sep 19, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

  • Ask an admin to enable usage-based reviews

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Next included review available in 58 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 78 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

The included review limit has been reached and this organization has disabled usage-based review continuation. Wait for reviews to reset or ask a billing admin to change After included review limits.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: b2d7fc80-2e0b-4f7f-ab03-a2d2a248efc8

📥 Commits

Reviewing files that changed from the base of the PR and between 0895876 and 649c039.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (8)
  • README.md
  • docs/command-research/calls.md
  • package.json
  • src/commands/sip-trunk/create.ts
  • src/commands/sip-trunk/endpoint.ts
  • src/commands/sip-trunk/rotate-credentials.ts
  • src/services/sip-trunks.ts
  • test/commands/sip-trunks.test.ts

Comment @coderabbitai help to get the list of available commands.

@coderabbitai

coderabbitai Bot commented Sep 19, 2026 •

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@purpshell

Copy link
Copy Markdown
Member Author

Monorepo contract PR: polymorfa/polymorfa#229. SDK: polymorfa/sdks#277.

@purpshell

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 19, 2026 •

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

…tial address

The API contract is now a discriminated union: a hosted address always has a
host, at least one transport and an RTP range. The CLI type mirrors it and the
decoder refuses a hosted response that lacks either.
@purpshell

Copy link
Copy Markdown
Member Author

Follow-up after review of polymorfa/polymorfa#229: the API response is now a discriminated union on status, so the CLI's SipEndpoint type is a union (SipEndpointHosted | SipEndpointNotHosted) and the decoder refuses a hosted response without transports or an RTP range. New test covers both partial shapes. Format, lint, typecheck, 491 tests and build pass.

@purpshell

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-22T13:15:18.526817Z 649c039 Manual request
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d2ea77f530

ℹ️ 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".

Comment thread src/commands/sip-trunk/create.ts Outdated
Comment on lines +38 to +42
const endpoint =
result.data.inboundCredentials !== undefined &&
this.outputMode(flags) === 'human'
? await trySipEndpoint(client)
: undefined

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Print one-time credentials before the endpoint lookup

When trunk creation succeeds but the subsequent optional endpoint request hangs or the process is interrupted, the server-side mutation has already completed while writeValue has not run, so the only copy of the inbound password is lost; credential rotation has the same ordering problem and can additionally leave the PBX without its new password after the old one expires. Emit the one-time credentials before awaiting this auxiliary lookup, then append the endpoint information if it becomes available.

Useful? React with 👍 / 👎.

…address

create and rotate-credentials awaited the optional address lookup before
writing anything, so a hung lookup or an interrupt lost the only copy of the
password. They now print the credentials first and append the address when
the lookup returns. A child-process test holds the lookup open and checks the
password is already on stdout.
@purpshell

Copy link
Copy Markdown
Member Author

Codex P1 (create.ts:42) is valid and fixed in e5839d8. sip-trunk create and rotate-credentials now write the one-time credentials before awaiting the optional SIP address lookup, then append the address block when it returns. Human output is unchanged when the lookup succeeds.

New test: a child process runs each command against a server that answers the trunk request and never answers /platform/sip/endpoint. The test asserts the password is already on stdout while the command is still running. With the previous ordering both cases time out after 20 s; with the fix both pass.

Checks: format, lint, typecheck, build, 493 tests, npm pack --dry-run, npm audit --omit=dev (0).

@purpshell

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

Reviewed commit: 670df0f1d3

ℹ️ 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".

@purpshell

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

Reviewed commit: 0972a0ed51

ℹ️ 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".

Copy link
Copy Markdown
Member Author

@codex review

Current head: e52545f. Optional SIP address lookup now stops after five seconds without retries. Both create and rotate print the one-time credential before that lookup; real HTTP child-process regressions prove they exit successfully when it hangs. Full local verify and production dependency audit passed. The exact SDK 0.1.0-dev.20260922115749 artifact is verified against SDK #277 and its successful publication run; dev is an ancestor.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e52545f8bd

ℹ️ 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".

Comment thread src/services/sip-trunks.ts Outdated
await getSipEndpoint(client, {
maxNetworkRetries: 0,
signal: AbortSignal.timeout(SIP_ENDPOINT_APPENDIX_TIMEOUT_MS),
timeoutMs: SIP_ENDPOINT_APPENDIX_TIMEOUT_MS,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve shorter user-specified request timeouts

When create or rotate-credentials is invoked with --timeout below 5000 and the credential mutation completes but the endpoint lookup hangs, this per-request timeoutMs overrides the timeout configured on the client, so the command can wait the full five seconds despite the global flag promising to fail requests after the specified duration. Keep the client's existing timeout and use the abort signal only as a five-second upper bound, or pass the smaller of the configured timeout and this cap.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

@codex review

Current head: 649c039. The optional SIP address lookup now respects a shorter global --timeout while retaining its five-second upper bound. Real child-process tests for create and rotate-credentials reproduced the 500 ms timeout being overridden before the fix; all 13 SIP command tests, typecheck, lint and formatting pass. API/SDK payloads, Admin, feature flags and availability are unchanged; README and command research describe the timeout behavior. The exact SDK pin remains 0.1.0-dev.20260922115749.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🎉

Reviewed commit: 649c0392b0

ℹ️ 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".

Copy link
Copy Markdown
Member Author

Merge gate exception approved by the owner for this PR: merge with green current-head CI and clean current-head Codex review, recording that CodeRabbit did not complete because its included bot review is rate-limited and the CLI review connection fails. No credits or billing changes were used. Final head: 649c039; target dev is included; CI run 35731852870 passed and Codex reviewed this exact head with no findings.

@purpshell
purpshell merged commit 18df902 into dev Sep 22, 2026
3 checks passed
@purpshell
purpshell deleted the t3code/calls-sip-address branch October 26, 2026 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant