Skip to content

[DX-964] Add missing spaces commands#175

Closed
sacOO7 wants to merge 6 commits intofix/ably-spaces-command-group-formattingfrom
feature/add-missing-spaces-commands
Closed

[DX-964] Add missing spaces commands#175
sacOO7 wants to merge 6 commits intofix/ably-spaces-command-group-formattingfrom
feature/add-missing-spaces-commands

Conversation

@sacOO7
Copy link
Contributor

@sacOO7 sacOO7 commented Mar 17, 2026

Added following missing commands

  1. spaces create
  2. spaces get
  3. spaces subscribe
  4. spaces members get-all

Summary by CodeRabbit

Release Notes

  • New Features

    • Added spaces create command to create new spaces.
    • Added spaces get command to view current space state and members.
    • Added spaces subscribe command to listen for real-time space updates.
    • Added spaces members get-all command to retrieve all space members.
  • Documentation

    • Updated README with usage guides and examples for all new space commands.
  • Bug Fixes

    • Improved space cleanup handling to prevent unhandled rejections during shutdown.

@vercel
Copy link

vercel bot commented Mar 17, 2026

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

Project Deployment Actions Updated (UTC)
cli-web-cli Ready Ready Preview, Comment Mar 17, 2026 9:52am

Request Review

@coderabbitai
Copy link

coderabbitai bot commented Mar 17, 2026

Walkthrough

The pull request introduces five new CLI commands for managing Ably Spaces (create, get, subscribe, members:get-all), enhanced cleanup logic in the base command to prevent unhandled rejections, comprehensive test suites covering all new commands, and updated documentation and mock utilities supporting the new functionality.

Changes

Cohort / File(s) Summary
New Spaces Commands
src/commands/spaces/create.ts, src/commands/spaces/get.ts, src/commands/spaces/subscribe.ts, src/commands/spaces/members/get-all.ts
Four new CLI commands added: create initializes a new space; get fetches and displays space members via REST presence API; subscribe listens for real-time space updates with member details; members:get-all retrieves all members in a space. Each command supports JSON output and includes error handling.
Command Infrastructure
src/commands/spaces/index.ts, src/spaces-base-command.ts
Index metadata expanded with new usage examples. Base command cleanup enhanced with unhandledRejection suppression, presence handler unsubscribe, polling for member disconnection (up to 10 seconds), and 200ms settlement delay.
Test Utilities
test/helpers/mock-ably-spaces.ts
MockSpace extended with public subscribe/unsubscribe methods and internal event emitter (\_emitter, \_emit) to enable space-level event triggering in tests.
Test Suites
test/unit/commands/spaces/create.test.ts, test/unit/commands/spaces/get.test.ts, test/unit/commands/spaces/members/get-all.test.ts, test/unit/commands/spaces/subscribe.test.ts
Four comprehensive test suites added covering space creation, member retrieval, subscription workflows, JSON output validation, error scenarios, and human-readable formatting. Total: ~499 lines of test code.
Documentation
README.md
Updated with new command entries, usage blocks, arguments, flags, examples, and navigation for all new Spaces commands, integrating them into the existing command index.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 Five new commands we've sewn with care,
Space create, get, subscribe through the air,
Members galore and cleanup refined,
Tests and examples, perfectly aligned! ✨
The warren of Spaces, now yours to share! 🎉

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'DX-964 Add missing spaces commands' directly and accurately reflects the main objective of the PR: implementing four new CLI commands for Ably Spaces (create, get, subscribe, members get-all).
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/add-missing-spaces-commands
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@sacOO7 sacOO7 changed the title Feature - Add missing spaces commands [DX-964] Add missing spaces commands Mar 17, 2026
@sacOO7 sacOO7 marked this pull request as ready for review March 18, 2026 09:46
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/spaces-base-command.ts (1)

89-117: ⚠️ Potential issue | 🟠 Major

Serialize the member cleanup poll instead of fire-and-forget intervals.

setInterval(() => { getAll(); }, 1000) can start a second members.getAll() before the first one settles. With no error handling in the getAll() function, a slow or rejected poll can then outlive this wait block and surface after the suppression handler has been removed at line 130, which reintroduces the shutdown failure this code is trying to avoid.

🧹 Nitpick comments (3)
src/commands/spaces/members/get-all.ts (1)

44-87: Use a single this.fail() funnel in run().

The inner catch already turns members.getAll() failures into CLI errors. The outer catch then catches that thrown error and sends it through this.fail() again. One top-level catch is enough here.

♻️ Suggested simplification
-      try {
-        const members: SpaceMember[] = await this.space!.members.getAll();
-
-        if (this.shouldOutputJson(flags)) {
-          this.logJsonResult(
-            {
-              members: members.map((member) => formatMemberOutput(member)),
-            },
-            flags,
-          );
-        } else if (members.length === 0) {
-          this.log(formatWarning("No members currently in this space."));
-        } else {
-          this.log(
-            `\n${formatHeading("Current members")} (${formatCountLabel(members.length, "member")}):\n`,
-          );
-
-          for (let i = 0; i < members.length; i++) {
-            const member = members[i];
-            this.log(`${formatIndex(i + 1)}`);
-            this.log(formatMemberBlock(member, { indent: "  " }));
-            this.log("");
-          }
-        }
-      } catch (error) {
-        this.fail(error, flags, "memberGetAll", { spaceName });
+      const members: SpaceMember[] = await this.space!.members.getAll();
+
+      if (this.shouldOutputJson(flags)) {
+        this.logJsonResult(
+          {
+            members: members.map((member) => formatMemberOutput(member)),
+          },
+          flags,
+        );
+      } else if (members.length === 0) {
+        this.log(formatWarning("No members currently in this space."));
+      } else {
+        this.log(
+          `\n${formatHeading("Current members")} (${formatCountLabel(members.length, "member")}):\n`,
+        );
+
+        for (let i = 0; i < members.length; i++) {
+          const member = members[i];
+          this.log(`${formatIndex(i + 1)}`);
+          this.log(formatMemberBlock(member, { indent: "  " }));
+          this.log("");
+        }
       }

As per coding guidelines "Use this.fail() as the single error funnel for all fatal errors in command run() methods".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/commands/spaces/members/get-all.ts` around lines 44 - 87, The nested
try/catch in run causes duplicate calls to this.fail; remove the inner catch and
let the outer catch be the single error funnel. Specifically, keep the
initializeSpace(...) and the members retrieval via this.space!.members.getAll()
inside the same try block, remove the inner catch that calls this.fail(error,
flags, "memberGetAll", { spaceName }), and let the outer catch handle errors
from initializeSpace or members.getAll by calling this.fail(error, flags,
"memberGetAll", { spaceName }). Ensure logic that logs JSON or formatted output
(shouldOutputJson, formatMemberOutput, formatMemberBlock, etc.) remains
unchanged.
test/unit/commands/spaces/create.test.ts (1)

38-49: Use the NDJSON helper for JSON-output assertions.

JSON.parse(stdout) is brittle once the command emits more than one JSON envelope. captureJsonLogs() is the project pattern for unit tests and keeps these assertions stable.

Based on learnings "For JSON output assertions, use the captureJsonLogs() helper from test/helpers/ndjson.ts."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/unit/commands/spaces/create.test.ts` around lines 38 - 49, The test
currently parses stdout with JSON.parse which breaks if multiple NDJSON
envelopes are emitted; replace JSON.parse(stdout) with the NDJSON helper
captureJsonLogs(stdout) from test/helpers/ndjson.ts, call it on the runCommand
stdout (same test that uses runCommand(["spaces:create", "test-space",
"--json"], import.meta.url)), then select the first envelope (e.g., const [json]
= captureJsonLogs(stdout)) and run the same assertions (json.type, json.success,
json.space.name === "test-space"); ensure captureJsonLogs is imported where
needed and remove the JSON.parse usage.
test/unit/commands/spaces/subscribe.test.ts (1)

97-107: Use NDJSON capture helper instead of JSON.parse(stdout) for this streaming unit test.

For subscribe-style unit tests, assert JSON records via the NDJSON helper to avoid brittle single-object parsing assumptions.

🧪 Suggested assertion update
 import { getMockAblyRealtime } from "../../../helpers/mock-ably-realtime.js";
+import { captureJsonLogs } from "../../../helpers/ndjson.js";
 // ...

-      const result = JSON.parse(stdout);
-      expect(result.type).toBe("event");
-      expect(result.members).toBeDefined();
-      expect(result.members).toBeInstanceOf(Array);
-      expect(result.members[0].clientId).toBe("user-1");
+      const records = captureJsonLogs(stdout);
+      const eventRecord = records.find((r) => r.type === "event");
+      expect(eventRecord).toBeDefined();
+      expect(eventRecord).toHaveProperty("members");
+      expect((eventRecord!.members as unknown[])).toBeInstanceOf(Array);
+      expect(
+        (eventRecord!.members as Record<string, unknown>[])[0],
+      ).toHaveProperty("clientId", "user-1");

Based on learnings: In unit tests for mocked streaming commands, assert stdout and use captureJsonLogs() for JSON output assertions.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/unit/commands/spaces/subscribe.test.ts` around lines 97 - 107, Replace
the brittle JSON.parse(stdout) assertion with the NDJSON capture helper: call
the test helper captureJsonLogs (or similar NDJSON helper used in tests) on
stdout returned from runCommand(["spaces:subscribe", "test-space", "--json"],
import.meta.url) to parse streaming JSON records, then assert the first record's
properties (type, members array, members[0].clientId) from the captured array;
update the assertions that reference result to instead use captured[0] and
ensure you import or reference captureJsonLogs in this test file if not already
present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@README.md`:
- Line 3724: Add the missing fenced-code language identifier "text" to each
newly added triple-backtick block so markdownlint MD040 is satisfied;
specifically replace the opening backticks for the usage examples for the CLI
commands shown (the blocks beginning "USAGE" for "ably spaces create", "ably
spaces get", "ably spaces members get-all", "ably spaces subscribe" and the
other occurrences noted) with ```text so each code fence is ```text ... ```
instead of just ```.

In `@src/commands/spaces/create.ts`:
- Around line 41-65: The command reports "Space created successfully" after only
calling initializeSpace(...) with enterSpace: false, which merely builds a
client and fetches a local Space instance; fix by invoking the actual
server-side create/publish operation before logging success (e.g., call the
service method that performs creation—look for functions like createSpace,
Space.create, or the client method used by src/commands/spaces/get.ts to
validate existence—and await its result), ensure the created Space has required
members/metadata (match the validation used by spaces.get), handle and surface
any errors, and only call logCliEvent/logJsonResult/formatSuccess after the real
create call completes successfully.

In `@src/commands/spaces/subscribe.ts`:
- Around line 91-100: The space-level "update" listener subscribed via
this.space!.subscribe("update", this.listener) must be removed before shutdown
to avoid late events writing to stdout/JSON; unsubscribe the listener (e.g.,
this.space!.unsubscribe("update", this.listener) or equivalent removeListener
API) after waitAndTrackCleanup(flags, "space", flags.duration) and/or add the
unsubscribe to SpacesBaseCommand.finally() cleanup so the listener is always
removed on error or exit.

In `@src/spaces-base-command.ts`:
- Around line 53-54: The temporary global handler registered with
process.on("unhandledRejection", onUnhandledRejection) must be unregistered in
an unconditional finally so it cannot remain installed if super.finally(error)
throws; update the finally/cleanup logic in the class (the method calling
process.on(...) and the override of finally — i.e. the code that calls
super.finally(error)) to ensure you call
process.removeListener("unhandledRejection", onUnhandledRejection) inside a
finally block that always runs (e.g., try { await super.finally(error) } finally
{ process.removeListener(...)}), and apply the same change to the other handler
cleanup region referenced (the block around lines 125–130) so both temporary
handlers are removed even on errors.

In `@test/unit/commands/spaces/subscribe.test.ts`:
- Around line 73-108: The test introduced an extra inner suite named
describe("JSON output") which breaks the rule requiring exactly five inner
describe blocks; move the it("should output JSON event for space updates"...)
test body into the existing "functionality" describe block (remove the
describe("JSON output") wrapper) and keep the test's setup (using
getMockAblySpaces, space.subscribe mockImplementation, and the runCommand call)
intact so the test still asserts JSON output via JSON.parse(stdout) and the same
expects; ensure no other describe blocks are added or removed so the file
retains the five required suites ('help', 'argument validation',
'functionality', 'flags', 'error handling').

---

Nitpick comments:
In `@src/commands/spaces/members/get-all.ts`:
- Around line 44-87: The nested try/catch in run causes duplicate calls to
this.fail; remove the inner catch and let the outer catch be the single error
funnel. Specifically, keep the initializeSpace(...) and the members retrieval
via this.space!.members.getAll() inside the same try block, remove the inner
catch that calls this.fail(error, flags, "memberGetAll", { spaceName }), and let
the outer catch handle errors from initializeSpace or members.getAll by calling
this.fail(error, flags, "memberGetAll", { spaceName }). Ensure logic that logs
JSON or formatted output (shouldOutputJson, formatMemberOutput,
formatMemberBlock, etc.) remains unchanged.

In `@test/unit/commands/spaces/create.test.ts`:
- Around line 38-49: The test currently parses stdout with JSON.parse which
breaks if multiple NDJSON envelopes are emitted; replace JSON.parse(stdout) with
the NDJSON helper captureJsonLogs(stdout) from test/helpers/ndjson.ts, call it
on the runCommand stdout (same test that uses runCommand(["spaces:create",
"test-space", "--json"], import.meta.url)), then select the first envelope
(e.g., const [json] = captureJsonLogs(stdout)) and run the same assertions
(json.type, json.success, json.space.name === "test-space"); ensure
captureJsonLogs is imported where needed and remove the JSON.parse usage.

In `@test/unit/commands/spaces/subscribe.test.ts`:
- Around line 97-107: Replace the brittle JSON.parse(stdout) assertion with the
NDJSON capture helper: call the test helper captureJsonLogs (or similar NDJSON
helper used in tests) on stdout returned from runCommand(["spaces:subscribe",
"test-space", "--json"], import.meta.url) to parse streaming JSON records, then
assert the first record's properties (type, members array, members[0].clientId)
from the captured array; update the assertions that reference result to instead
use captured[0] and ensure you import or reference captureJsonLogs in this test
file if not already present.
🪄 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: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 9ce1f70d-8b2b-4d4a-b26c-e37e9f641ae4

📥 Commits

Reviewing files that changed from the base of the PR and between 02b7c9c and 0e10d5a.

📒 Files selected for processing (12)
  • README.md
  • src/commands/spaces/create.ts
  • src/commands/spaces/get.ts
  • src/commands/spaces/index.ts
  • src/commands/spaces/members/get-all.ts
  • src/commands/spaces/subscribe.ts
  • src/spaces-base-command.ts
  • test/helpers/mock-ably-spaces.ts
  • test/unit/commands/spaces/create.test.ts
  • test/unit/commands/spaces/get.test.ts
  • test/unit/commands/spaces/members/get-all.test.ts
  • test/unit/commands/spaces/subscribe.test.ts


Create a new space

```
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add a language identifier to newly added fenced code blocks (MD040).

These new fences are missing a language tag and will keep markdownlint warning on changed lines.

🛠️ Suggested lint fix
-```
+```text
 USAGE
   $ ably spaces create SPACE [-v] [--json | --pretty-json] [--client-id <value>]
 ...

- +text
USAGE
$ ably spaces get SPACE [-v] [--json | --pretty-json]
...


-```
+```text
USAGE
  $ ably spaces members get-all SPACE [-v] [--json | --pretty-json] [--client-id <value>]
...

- +text
USAGE
$ ably spaces subscribe SPACE [-v] [--json | --pretty-json] [--client-id ] [-D ]
...

Also applies to: 3890-3890, 4280-4280, 4345-4345

🧰 Tools
🪛 markdownlint-cli2 (0.21.0)

[warning] 3724-3724: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` at line 3724, Add the missing fenced-code language identifier
"text" to each newly added triple-backtick block so markdownlint MD040 is
satisfied; specifically replace the opening backticks for the usage examples for
the CLI commands shown (the blocks beginning "USAGE" for "ably spaces create",
"ably spaces get", "ably spaces members get-all", "ably spaces subscribe" and
the other occurrences noted) with ```text so each code fence is ```text ... ```
instead of just ```.

Comment on lines +41 to +65
await this.initializeSpace(flags, spaceName, {
enterSpace: false,
setupConnectionLogging: false,
});

this.logCliEvent(
flags,
"space",
"created",
"Space created successfully",
{ spaceName },
);

if (this.shouldOutputJson(flags)) {
this.logJsonResult(
{
space: {
name: spaceName,
},
},
flags,
);
} else {
this.log(formatSuccess(`Space ${formatResource(spaceName)} created.`));
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

This reports success after a local spaces.get() only.

initializeSpace(..., { enterSpace: false }) just creates a client and fetches a Space instance, then this command exits. Nothing here publishes, enters, or calls a real create operation, so users can get “Space created successfully” even though the new src/commands/spaces/get.ts implementation treats a space with no members as nonexistent.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/commands/spaces/create.ts` around lines 41 - 65, The command reports
"Space created successfully" after only calling initializeSpace(...) with
enterSpace: false, which merely builds a client and fetches a local Space
instance; fix by invoking the actual server-side create/publish operation before
logging success (e.g., call the service method that performs creation—look for
functions like createSpace, Space.create, or the client method used by
src/commands/spaces/get.ts to validate existence—and await its result), ensure
the created Space has required members/metadata (match the validation used by
spaces.get), handle and surface any errors, and only call
logCliEvent/logJsonResult/formatSuccess after the real create call completes
successfully.

Comment on lines +91 to +100
this.space!.subscribe("update", this.listener);

this.logCliEvent(
flags,
"space",
"subscribed",
"Subscribed to space updates",
);

await this.waitAndTrackCleanup(flags, "space", flags.duration);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Unsubscribe the space-level listener before shutdown.

SpacesBaseCommand.finally() cleans up members/locks/locations/cursors, but not space.subscribe(...). A late "update" event can still write to stdout/JSON after waitAndTrackCleanup() has already decided to exit.

♻️ Suggested cleanup
-      await this.waitAndTrackCleanup(flags, "space", flags.duration);
+      try {
+        await this.waitAndTrackCleanup(flags, "space", flags.duration);
+      } finally {
+        if (this.listener) {
+          this.space?.unsubscribe("update", this.listener);
+        }
+      }
📝 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
this.space!.subscribe("update", this.listener);
this.logCliEvent(
flags,
"space",
"subscribed",
"Subscribed to space updates",
);
await this.waitAndTrackCleanup(flags, "space", flags.duration);
this.space!.subscribe("update", this.listener);
this.logCliEvent(
flags,
"space",
"subscribed",
"Subscribed to space updates",
);
try {
await this.waitAndTrackCleanup(flags, "space", flags.duration);
} finally {
if (this.listener) {
this.space?.unsubscribe("update", this.listener);
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/commands/spaces/subscribe.ts` around lines 91 - 100, The space-level
"update" listener subscribed via this.space!.subscribe("update", this.listener)
must be removed before shutdown to avoid late events writing to stdout/JSON;
unsubscribe the listener (e.g., this.space!.unsubscribe("update", this.listener)
or equivalent removeListener API) after waitAndTrackCleanup(flags, "space",
flags.duration) and/or add the unsubscribe to SpacesBaseCommand.finally()
cleanup so the listener is always removed on error or exit.

Comment on lines +53 to +54
process.on("unhandledRejection", onUnhandledRejection);

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Always remove the temporary unhandledRejection handler in a finally.

process.removeListener(...) only runs after the whole cleanup path succeeds. If super.finally(error) throws, this global handler stays installed and will keep suppressing unrelated rejections for the rest of the process.

Also applies to: 125-130

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/spaces-base-command.ts` around lines 53 - 54, The temporary global
handler registered with process.on("unhandledRejection", onUnhandledRejection)
must be unregistered in an unconditional finally so it cannot remain installed
if super.finally(error) throws; update the finally/cleanup logic in the class
(the method calling process.on(...) and the override of finally — i.e. the code
that calls super.finally(error)) to ensure you call
process.removeListener("unhandledRejection", onUnhandledRejection) inside a
finally block that always runs (e.g., try { await super.finally(error) } finally
{ process.removeListener(...)}), and apply the same change to the other handler
cleanup region referenced (the block around lines 125–130) so both temporary
handlers are removed even on errors.

Comment on lines +73 to +108
describe("JSON output", () => {
it("should output JSON event for space updates", async () => {
const spacesMock = getMockAblySpaces();
const space = spacesMock._getSpace("test-space");

space.subscribe.mockImplementation(
(event: string, cb: (state: unknown) => void) => {
setTimeout(() => {
cb({
members: [
{
clientId: "user-1",
connectionId: "conn-1",
isConnected: true,
profileData: { name: "Alice" },
location: null,
lastEvent: { name: "enter", timestamp: 1710000000000 },
},
],
});
}, 10);
},
);

const { stdout } = await runCommand(
["spaces:subscribe", "test-space", "--json"],
import.meta.url,
);

const result = JSON.parse(stdout);
expect(result.type).toBe("event");
expect(result.members).toBeDefined();
expect(result.members).toBeInstanceOf(Array);
expect(result.members[0].clientId).toBe("user-1");
});
});
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Keep exactly five inner describe blocks in this unit test file.

describe("JSON output") introduces an extra inner suite. Please fold this test into the existing functionality block so this file keeps the required five inner sections only.

♻️ Suggested structure change
-  describe("functionality", () => {
+  describe("functionality", () => {
     it("should subscribe to space update events and output in block format", async () => {
       // ...
     });

     it("should subscribe to update events without entering space", async () => {
       // ...
     });
-  });
-
-  describe("JSON output", () => {
+
     it("should output JSON event for space updates", async () => {
       // ...
     });
   });

As per coding guidelines: test/unit/commands/**/*.test.ts: Each unit test file must have exactly 5 required describe blocks: 'help', 'argument validation', 'functionality', 'flags', and 'error handling'.

📝 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
describe("JSON output", () => {
it("should output JSON event for space updates", async () => {
const spacesMock = getMockAblySpaces();
const space = spacesMock._getSpace("test-space");
space.subscribe.mockImplementation(
(event: string, cb: (state: unknown) => void) => {
setTimeout(() => {
cb({
members: [
{
clientId: "user-1",
connectionId: "conn-1",
isConnected: true,
profileData: { name: "Alice" },
location: null,
lastEvent: { name: "enter", timestamp: 1710000000000 },
},
],
});
}, 10);
},
);
const { stdout } = await runCommand(
["spaces:subscribe", "test-space", "--json"],
import.meta.url,
);
const result = JSON.parse(stdout);
expect(result.type).toBe("event");
expect(result.members).toBeDefined();
expect(result.members).toBeInstanceOf(Array);
expect(result.members[0].clientId).toBe("user-1");
});
});
describe("functionality", () => {
it("should subscribe to space update events and output in block format", async () => {
// existing test code...
});
it("should subscribe to update events without entering space", async () => {
// existing test code...
});
it("should output JSON event for space updates", async () => {
const spacesMock = getMockAblySpaces();
const space = spacesMock._getSpace("test-space");
space.subscribe.mockImplementation(
(event: string, cb: (state: unknown) => void) => {
setTimeout(() => {
cb({
members: [
{
clientId: "user-1",
connectionId: "conn-1",
isConnected: true,
profileData: { name: "Alice" },
location: null,
lastEvent: { name: "enter", timestamp: 1710000000000 },
},
],
});
}, 10);
},
);
const { stdout } = await runCommand(
["spaces:subscribe", "test-space", "--json"],
import.meta.url,
);
const result = JSON.parse(stdout);
expect(result.type).toBe("event");
expect(result.members).toBeDefined();
expect(result.members).toBeInstanceOf(Array);
expect(result.members[0].clientId).toBe("user-1");
});
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/unit/commands/spaces/subscribe.test.ts` around lines 73 - 108, The test
introduced an extra inner suite named describe("JSON output") which breaks the
rule requiring exactly five inner describe blocks; move the it("should output
JSON event for space updates"...) test body into the existing "functionality"
describe block (remove the describe("JSON output") wrapper) and keep the test's
setup (using getMockAblySpaces, space.subscribe mockImplementation, and the
runCommand call) intact so the test still asserts JSON output via
JSON.parse(stdout) and the same expects; ensure no other describe blocks are
added or removed so the file retains the five required suites ('help', 'argument
validation', 'functionality', 'flags', 'error handling').

@umair-ably umair-ably force-pushed the fix/ably-spaces-command-group-formatting branch from 7451a66 to 93e0e2a Compare March 18, 2026 18:49
@sacOO7 sacOO7 force-pushed the fix/ably-spaces-command-group-formatting branch from 326550a to 39199d4 Compare March 19, 2026 09:40
@sacOO7
Copy link
Contributor Author

sacOO7 commented Mar 19, 2026

Closing in favor of #177

@sacOO7 sacOO7 closed this Mar 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant