[DX-1101] Fix keys revoke to use correct Control API endpoint

[sacOO7]

• Fixes https://ably.atlassian.net/browse/DX-1101
• Fixed revokeKey to use POST /apps/{app_id}/keys/{key_id}/revoke instead of
  DELETE /apps/{app_id}/keys/{key_id}, matching the actual Control API spec
• Handle empty 200 response body from the revoke endpoint (head :ok)
• Updated unit test mocks to match the corrected endpoint
• Seems E2E test for this will be addressed as a part of [DX-967] Adds more happy path E2E tests #250

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
cli-web-cli	Ready	Preview, Comment	Apr 17, 2026 2:11pm

[claude-code-ably-assistant]

Walkthrough

This PR fixes a bug where the revokeKey method in the Control API service was calling DELETE /apps/{app_id}/keys/{key_id} instead of the correct POST /apps/{app_id}/keys/{key_id}/revoke. It also adds defensive handling for the revoke endpoint's empty 200 response body (using response.text() before JSON.parse rather than response.json() directly). Unit test mocks are updated to match the corrected endpoint.

Changes

Area	Files	Summary
Services	src/services/control-api.ts	Changed revokeKey from DELETE .../keys/{id} to POST .../keys/{id}/revoke; added empty-body guard in shared response parser
Tests	test/unit/commands/auth/keys/revoke.test.ts	Updated all three nock interceptors from .delete() to .post(...revoke)
Tests	test/unit/services/control-api.test.ts	Updated error-path nock interceptor from .delete() to .post(...revoke)

Review Notes

• Behavioral fix: The previous DELETE call would have been rejected by the Control API (wrong HTTP method/path), meaning all key revocations were broken before this change.
• Empty body handling: The revoke endpoint returns an empty 200 OK. The original response.json() call throws on an empty body; the fix reads via response.text() first and skips parsing when the string is empty. This change is in the shared response-parsing path in ControlApi, so any other endpoint that returns an empty body will now also be handled safely.
• No E2E coverage yet: Per the PR description, E2E test coverage for this flow will be addressed separately in [DX-967] Adds more happy path E2E tests #250.
• No new dependencies.

[claude-code-ably-assistant]

Review Summary

This PR fixes a real bug: revokeKey was calling DELETE /apps/{appId}/keys/{keyId} when the Control API spec requires POST /apps/{appId}/keys/{keyId}/revoke. The endpoint fix and test updates are correct.

One real gap to address:

The empty-body fix is untested. The PR description says the revoke endpoint returns 200 OK with an empty body, and the fix handles that case in request(). But all three test files mock with reply(200, {}) which sends non-empty JSON. The new empty-body branch (if (!text) return {} as T) is never exercised by any test.

If someone later changed the empty-body guard, the existing tests would still pass, masking the regression. To fix: add a success test in control-api.test.ts that uses reply(200, '') to simulate the real API behavior, and update the command-level mocks from reply(200, {}) to reply(200, '') as well.

Minor robustness note (non-blocking): if (!text) does not guard against whitespace-only bodies. if (!text.trim()) is a safer guard, though in practice the Control API returns empty string not whitespace.

Everything else looks good:

• Endpoint is correct per spec (POST .../revoke)
• response.text() then JSON.parse is the right pattern for handling optional response bodies
• The 204 early-return is preserved and unaffected
• All three test files consistently updated
• revoke.ts command logic did not need to change

[AndyTWF]

(Just requesting changes whilst the wider question is resolved, not because I have an issue with the code).

Reading the Control API refs, every endpoint other than keys has its delete operation as HTTP DELETE. Keys are the only exception.

So it begs the question:

• Does DELETE keys/foo and POST keys/foo/revoke produce the same result?
• If the answer to above is "yes", should we actually be updating the Control API documentation and therefore making DELETE the consistent verb for these types of operation?

[sacOO7]

(Just requesting changes whilst the wider question is resolved, not because I have an issue with the code).

Reading the Control API refs, every endpoint other than keys has its delete operation as HTTP DELETE. Keys are the only exception.

So it begs the question:

• Does DELETE keys/foo and POST keys/foo/revoke produce the same result?
• If the answer to above is "yes", should we actually be updating the Control API documentation and therefore making DELETE the consistent verb for these types of operation?

Maybe it's a bug or design decision from control API team. But normal DELETE doesn't work as expected : (
PS. I checked source code with claude and no end-point found with DELETE keys
@kennethkalmer thoughts ?

Found internal discussion -> https://ably-real-time.slack.com/archives/CB285667N/p1614955572251400