fix(sip): QA blockers and high-severity defects - #91
Merged
Merged
Conversation
Blockers: - delete ran its dependency read only on the refusal path, so --yes deleted silently. Deleting an in-use URI cascade-deletes the trunks using it, which is what that read exists to surface. - trunks update never sent trunk_direction, which the API requires on every update, so every flag but --status and --secure 400'd. - diagnose exited 0 on failure, and the assistant filed support tickets and offered console-only remedies to terminal users. High: - -o json ignored by every update and delete - trunk_domain missing from create -o json - credentials update presented a required password flag as optional - uris create took a password on argv - numbers update --trunk-id skipped its guard under --dry-run
Third instance of the same API shape: a write is refused unless it restates a field it is not changing. The URI update demands both authentication_needed=true and a username alongside a password, so --password-stdin alone always failed. Both are now carried across.
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.
Fixes all 5 blockers and all 5 highs from the V2 QA pass. Mediums and the low are deliberately left for a follow-up.
Blockers
1 ·
sip * deleteran its dependency check only when refusing. With--yesthere was no pre-flight read at all. Worse than reported: deleting an in-use URI cascade-deletes the trunks pointing at it — I reproduced this on a live account and destroyed a real trunk with it. The read now always runs;--yesskips the confirmation only.2 ·
sip trunks update400'd on every flag except--statusand--secure. The API requirestrunk_directionon every update and the CLI never sent it. Read from the trunk now, with--directionas an override.3 ·
diagnoseexited 0 when it failed. The stream succeeds and carries a failure message, so there was no protocol-level error. It now keys off the assistant ending the turn inescalate_to_support— an investigation that files a ticket did not diagnose anything. Not prose matching.4 · The ticket claim was true. The tool stream shows
escalate_to_supportactually running, so everysip calls diagnoseon a trunk call files a real support ticket. Ruled out in the request; the durable fix is in the debugger prompt.5 · "Reload the Plivo Console" to a terminal user. Same mechanism.
High
-o jsonignored by everyupdate/delete— empty stdout, prose on stderr, exit 0trunk_domainmissing fromcreate -o json, present in table modecredentials updatepresented--password-stdinas optional; the API rewrites the password every update, so omitting it blanks itnumbers update --trunk-idskipped its outbound guard under--dry-run, so the preview showed a request the real run refusesuris createtook--passwordon argv. Now stdin-only like credentials, and rotatable on updateTesting
Full suite 17 packages / 0 fail, golangci-lint 0 issues. Blockers 1, 2 and 9 verified live against an account with real trunk traffic.
Every fix has a negative control — breaking it fails the matching test. One control initially passed because gofmt had realigned my anchor; re-run with a regex anchor, it fails correctly.
Not in this PR
Mediums 11-15 (client-side validation, enum rejection, error rendering,
--limit/--all,objects: null) and upstream 16 (trunks listomitting a trunkgetreturns — reproducible through the raw API, so not a CLI bug).