Conversation
Refactors documentation and `.cursor` rules to improve clarity, consistency, and effectiveness for AI agent interactions, based on analysis of recent development history (`.specstory`) and the corresponding TODO item. Key changes include: - **Created `.cursor/rules/WORKFLOW.mdc`:** Centralizes the mandatory 4-step development workflow (Build, Lint, Test, Document) required for all changes. - **Refactored `.cursor/rules`:** Updated `development.mdc`, `ably.mdc`, and `project.mdc` to remove redundancy, improve focus, add specific guidance (linting philosophy, Dockerfile tips), and link to the new `WORKFLOW.mdc` and relevant docs. - **Enhanced `docs/Testing.md`:** Added clearer instructions for running and debugging different test types (Unit, Integration, E2E, Playwright), emphasizing E2E/Integration preference for core Ably features and addressing common pitfalls observed in test runs. - **Added `CONTRIBUTING.md`:** Formalized the contribution process, emphasizing the mandatory workflow (file moved to root directory). - **Added `docs/DEBUGGING.md`:** Provided initial tips for troubleshooting common test failures and local CLI issues. - **Updated `README.md`:** Added a link to the new `CONTRIBUTING.md` file. - **Updated `docs/TODO.md`:** Marked the documentation review task as complete.
- Create agent-agnostic .cursor/rules/AI_ASSISTANCE.mdc with common patterns and code examples - Add detailed TROUBLESHOOTING.md with solutions for common errors - Enhance Testing.md with better visual hierarchy and progressive disclosure - Move CONTRIBUTING.md to root directory following standard conventions - Update README.md with link to CONTRIBUTING.md
Standardized file naming conventions across: .cursor/rules: ABLY.mdc → Ably.mdc .cursor/rules: DEVELOPMENT.mdc → Development.mdc .cursor/rules: PROJECT.mdc → Project.mdc .cursor/rules: WORKFLOW.mdc → Workflow.mdc docs: DEBUGGING.md → Debugging.md Updated all cross-references to maintain consistency.
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update introduces a comprehensive suite of documentation and workflow enhancements for the Ably CLI project. New guideline documents for AI assistance, contribution, debugging, and troubleshooting are added, alongside a detailed and structured rewrite of the testing policy. A mandatory development workflow is formalized in both a dedicated rule file and the main README, ensuring contributors follow standardized build, lint, test, and documentation update steps. The TODO list is updated to mark documentation improvements as complete, reflecting the recent refactor and expansion of project guidance materials. Changes
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (12)
README.md (1)
4746-4746: Move and hyphenate terminal server descriptionThe line about the terminal server is currently placed under the Contributing section—it belongs under the # Terminal Server section. Also, hyphenate “web-based CLI” for clarity:
- This repository includes a terminal server that provides a bash environment over WebSockets for the web based CLI. See [Server-Setup.md](./docs/Server-Setup.md) for more info on how it works and how to deploy it. + This repository includes a terminal server that provides a bash environment over WebSockets for the web-based CLI. See [Server-Setup.md](./docs/Server-Setup.md) for more info on how it works and how to deploy it.🧰 Tools
🪛 LanguageTool
[uncategorized] ~4746-~4746: This expression is usually spelled with a hyphen.
Context: ...ash environment over WebSockets for the web based CLI. See [Server-Setup.md](./docs/Serve...(BASED_HYPHEN)
docs/TODO.md (1)
98-100: Nit: Normalize nested list indentation and spacing.The completed item and its sub-bullets use inconsistent spacing (
" - *Done"). For readability and markdown lint compliance, collapse extra spaces:- - *Done: 2025-04-27* - - *Summary: Refactored `.cursor/rules`, updated `docs/Testing.md`, added `CONTRIBUTING.md` and `docs/DEBUGGING.md` based on analysis of the `.specstory` files and Anthropic best practices. Added code examples, troubleshooting guide, and agent-agnostic AI assistance rules. Enhanced visual hierarchy and progressive disclosure in documentation. Created `WORKFLOW.mdc` to centralize mandatory steps.* + - *Done: 2025-04-27* + - *Summary: Refactored `.cursor/rules`, updated `docs/Testing.md`, added `CONTRIBUTING.md` and `docs/DEBUGGING.md` based on analysis of the `.specstory` files and Anthropic best practices. Added code examples, troubleshooting guide, and agent-agnostic AI assistance rules. Enhanced visual hierarchy and progressive disclosure in documentation. Created `WORKFLOW.mdc` to centralize mandatory steps.*CONTRIBUTING.md (2)
11-14: Add language specifier to fenced code blocks.The build, lint, test, and document steps are in fenced blocks without a language. Mark them as
bash(orsh) to satisfy markdownlint (MD040):- ``` + ```bash
1-41: Ensure trailing newline at end of file.It's best practice to end markdown files with a single newline to avoid diff noise in future edits.
.cursor/rules/AI-Assistance.mdc (1)
26-49: Specify languages for code fences.All TypeScript and shell examples should declare their language for better readability and to satisfy markdownlint (MD040):
- ``` + ```typescript ... (TypeScript examples) ... - ``` + ```bash ... (Shell examples) ...docs/Troubleshooting.md (1)
14-23: Specify languages for code fences.Error snippets and code examples currently lack language identifiers. For example:
- ``` + ```text ... error output ... - ``` + ```typescript ... import example ...🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
14-14: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
docs/Debugging.md (1)
19-64: Specify languages for all fenced code blocks.Several
```blocks need language tags (bash,typescript, etc.) to pass markdownlint and improve readability:- ```bash + ```bash - ```typescript + ```typescript - ```bash + ```bash🧰 Tools
🪛 LanguageTool
[uncategorized] ~24-~24: Loose punctuation mark.
Context: ..., VS Code debugger). *console.log: Add temporaryconsole.logstatement...(UNLIKELY_OPENING_PUNCTUATION)
🪛 markdownlint-cli2 (0.17.2)
26-26: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
27-27: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
28-28: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
30-30: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
31-31: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
32-32: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
33-33: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
38-38: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
39-39: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
40-40: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
48-48: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
49-49: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
50-50: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
51-51: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
.cursor/rules/Workflow.mdc (3)
1-5: Fill or remove incomplete frontmatter fields
The YAML frontmatter listsdescription:andglobs:without values. Consider populating these fields with a brief description and appropriate glob patterns, or removing them entirely if they’re not required by your tooling.
25-31: Unify indentation in the Step 4 checklist
The nested bullets under Update Documentation & Rules mix two- and four-space indents, which can render inconsistently. Consider standardizing on two spaces (or a single tab) for all list levels to ensure proper Markdown rendering.
35-35: Add trailing period to closing admonition
For consistency with other instructions, add a period at the end of "Only after successfully completing ALL four steps should you consider your task complete.".docs/Testing.md (2)
7-10: Unify link text for Workflow rules reference
Line 8 links to[Workflow.mdc](mdc:.cursor/rules/Workflow.mdc), while Line 26 uses[.cursor/rules/Workflow.mdc](mdc:.cursor/rules/Workflow.mdc). To maintain consistency, choose one link text (either the filename or full path) and apply it throughout the document.
136-140: Clarify sentence for complete context
The line “Run automatically in CI (GitHub Actions) on PRs and merges. Can be run locally but may incur costs.” is missing a subject in the second sentence. Consider rephrasing to:These tests run automatically in CI (GitHub Actions) on PRs and merges, and they can be run locally but may incur costs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.cursor/rules/AI-Assistance.mdc(1 hunks).cursor/rules/Workflow.mdc(1 hunks)CONTRIBUTING.md(1 hunks)README.md(1 hunks)docs/Debugging.md(1 hunks)docs/TODO.md(1 hunks)docs/Testing.md(1 hunks)docs/Troubleshooting.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~4746-~4746: This expression is usually spelled with a hyphen.
Context: ...ash environment over WebSockets for the web based CLI. See [Server-Setup.md](./docs/Serve...
(BASED_HYPHEN)
docs/Debugging.md
[uncategorized] ~24-~24: Loose punctuation mark.
Context: ..., VS Code debugger). * console.log: Add temporary console.log statement...
(UNLIKELY_OPENING_PUNCTUATION)
docs/Troubleshooting.md
[uncategorized] ~192-~192: It appears that hyphens are missing in the adjective “up-to-date”.
Context: ...on**: - Check that type definitions are up to date: ```bash pnpm install @types/node@l...
(UP_TO_DATE_HYPHEN)
docs/Testing.md
[style] ~141-~141: To form a complete sentence, be sure to include a subject.
Context: ... CI (GitHub Actions) on PRs and merges. Can be run locally but may incur costs. **...
(MISSING_IT_THERE)
🪛 markdownlint-cli2 (0.17.2)
docs/Debugging.md
26-26: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
27-27: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
28-28: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
30-30: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
31-31: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
32-32: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
33-33: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
38-38: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
39-39: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
40-40: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
48-48: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
49-49: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
50-50: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
51-51: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
docs/Troubleshooting.md
14-14: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
37-37: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
69-69: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
107-107: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
187-187: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test
- GitHub Check: e2e-tests
🔇 Additional comments (3)
README.md (2)
4748-4749: Heading level for ContributingThe
# Contributingheading at lines 4748-4749 correctly matches other major sections in this README. Please ensure the earlier## Contributingis removed so only this top-level heading remains.
4750-4751:Details
✅ Verification successful
Verify the link to the Contribution workflow
Ensure that
.cursor/rules/Workflow.mdcis the correct relative path and that this file is present in the repository. If the file is moved or structured differently, update the link accordingly.
🏁 Script executed:
#!/bin/bash # Verify that Workflow.mdc exists in the repository fd Workflow.mdcLength of output: 17
To locate the correct file (and verify that the path in README.md is accurate), let’s search for any files or directories matching “Workflow” and “.cursor” in the repo:
🏁 Script executed:
#!/bin/bash # List all files with “Workflow” (any extension, case-insensitive) find . -type f -iname "*workflow.*" | sort # Find any “.cursor” directory find . -type d -iname ".cursor" -print # If .cursor/rules exists, list its contents if [ -d ".cursor/rules" ]; then echo ".cursor/rules contents:" ls -1 .cursor/rules fiLength of output: 296
Link Verified:
.cursor/rules/Workflow.mdcExistsThe referenced file is present at the specified path, so the link in README.md is correct and requires no changes.
- Checked:
.cursor/rules/Workflow.mdc.cursor/rules/Workflow.mdc (1)
16-19: Verify reference todevelopment.mdc
Step 2 directs readers todevelopment.mdcfor linting philosophy, but that file doesn’t appear to exist in the repo. Please confirm the intended filename/path (e.g.,.cursor/rules/Development.mdcordocs/Development.md) and update the link to avoid a broken reference.
| ## Contributing | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove duplicate Contributing section or correct heading level
The snippet introduces two “Contributing” headings—## Contributing at lines 4744-4745 and # Contributing at lines 4748-4749. To maintain consistency with other top-level sections (which use #), remove the extra ## Contributing lines and rely on a single # Contributing section.
| * [Project Structure](mdc:docs/Project-Structure.md): Know where different code components live. | ||
| * [Testing Strategy](mdc:docs/Testing.md): Understand the different types of tests and how to run them. | ||
| * [Development Rules](mdc:.cursor/rules/development.mdc): Coding standards, linting, dependency management. | ||
| * [Ably Rules](mdc:.cursor/rules/ably.mdc): How to interact with Ably APIs/SDKs. | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the referenced rules file exists.
The link points to .cursor/rules/development.mdc, but this file isn’t part of the PR context. Ensure that development.mdc is in the repo, or update the link to the correct path (e.g., Workflow.mdc):
🏁 Script executed:
rg -l ".cursor/rules/development.mdc"Length of output: 53
🏁 Script executed:
#!/bin/bash
echo "Listing all .mdc files:"
find . -type f -name '*.mdc'
echo "Searching for development.mdc:"
find . -type f -name development.mdc
echo "Searching for ably.mdc:"
find . -type f -name ably.mdcLength of output: 434
Update CONTRIBUTING.md links to match actual filenames
The rules files exist as .cursor/rules/Development.mdc and .cursor/rules/Ably.mdc (capitalized), but the links use lowercase paths. On case-sensitive filesystems these will 404. Please update the links in CONTRIBUTING.md (around lines 23–27):
-
Change
[Development Rules](mdc:.cursor/rules/development.mdc)
to
[Development Rules](mdc:.cursor/rules/Development.mdc) -
Change
[Ably Rules](mdc:.cursor/rules/ably.mdc)
to
[Ably Rules](mdc:.cursor/rules/Ably.mdc)
| --- | ||
| description: | ||
| globs: | ||
| alwaysApply: false | ||
| --- |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Frontmatter is incomplete or unused.
The YAML frontmatter has empty description and globs keys. Either populate these fields with valid metadata or remove the frontmatter block if it’s not processed by your toolchain.
- ---
- description:
- globs:
- alwaysApply: false
- ---
+ ---
+ description: "AI assistance guidelines for contributing to the Ably CLI"
+ globs:
+ - "*.ts"
+ - "src/**/*.ts"
+ - "docs/**/*.md"
+ alwaysApply: true
+ ---📝 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.
| --- | |
| description: | |
| globs: | |
| alwaysApply: false | |
| --- | |
| --- | |
| description: "AI assistance guidelines for contributing to the Ably CLI" | |
| globs: | |
| - "*.ts" | |
| - "src/**/*.ts" | |
| - "docs/**/*.md" | |
| alwaysApply: true | |
| --- |
| * Verify mocks (`sinon`, `nock`) are correctly set up and restored (`beforeEach`/`afterEach`). | ||
| * Ensure stubs match the actual function signatures. | ||
| * Check that network requests are being intercepted as expected (e.g., using `nock.recorder`). | ||
| * **E2E Failures:** | ||
| * **Credentials:** Ensure `E2E_ABLY_API_KEY` (and any other required keys) are correctly set in your environment (`.env` file locally, secrets in CI). | ||
| * **Network:** Check for network connectivity issues to Ably services. | ||
| * **Resource Conflicts:** Ensure previous test runs cleaned up resources correctly (e.g., unique channel names per test run). | ||
| * **CI vs. Local:** If tests fail only in CI, suspect environment differences (Node version, dependencies, permissions, resources). Check CI logs carefully. | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Correct list indentation to two spaces.
Nested list items are indented with four spaces, which conflicts with markdownlint’s MD007. Normalize to two-space indents:
- * Verify mocks (`sinon`, `nock`) are correctly set up and restored (`beforeEach`/`afterEach`).
+ * Verify mocks (`sinon`, `nock`) are correctly set up and restored (`beforeEach`/`afterEach`).📝 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.
| * Verify mocks (`sinon`, `nock`) are correctly set up and restored (`beforeEach`/`afterEach`). | |
| * Ensure stubs match the actual function signatures. | |
| * Check that network requests are being intercepted as expected (e.g., using `nock.recorder`). | |
| * **E2E Failures:** | |
| * **Credentials:** Ensure `E2E_ABLY_API_KEY` (and any other required keys) are correctly set in your environment (`.env` file locally, secrets in CI). | |
| * **Network:** Check for network connectivity issues to Ably services. | |
| * **Resource Conflicts:** Ensure previous test runs cleaned up resources correctly (e.g., unique channel names per test run). | |
| * **CI vs. Local:** If tests fail only in CI, suspect environment differences (Node version, dependencies, permissions, resources). Check CI logs carefully. | |
| * Verify mocks (`sinon`, `nock`) are correctly set up and restored (`beforeEach`/`afterEach`). | |
| * Ensure stubs match the actual function signatures. | |
| * Check that network requests are being intercepted as expected (e.g., using `nock.recorder`). | |
| * **E2E Failures:** | |
| * **Credentials:** Ensure `E2E_ABLY_API_KEY` (and any other required keys) are correctly set in your environment (`.env` file locally, secrets in CI). | |
| * **Network:** Check for network connectivity issues to Ably services. | |
| * **Resource Conflicts:** Ensure previous test runs cleaned up resources correctly (e.g., unique channel names per test run). | |
| * **CI vs. Local:** If tests fail only in CI, suspect environment differences (Node version, dependencies, permissions, resources). Check CI logs carefully. |
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
26-26: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
27-27: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
28-28: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
30-30: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
31-31: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
32-32: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
33-33: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
| ```bash | ||
| # Run a specific test file | ||
| pnpm test test/unit/commands/apps/create.test.ts | ||
|
|
||
| ### Folder Structure | ||
| # Run all tests in a directory | ||
| pnpm test test/integration/core/**/*.test.ts | ||
| ``` |
There was a problem hiding this comment.
Ensure Bash snippet is properly fenced
The "Run Specific Files" section opens a bash block but the closing backticks seem misaligned. Verify that the snippet from Lines 37–43 starts with bash and ends with , so it renders correctly in Markdown.
See #10 - Always attach AI and Workflow cursor rules - Fix README typos
Rewritten for simplification
See #10 - Always attach AI and Workflow cursor rules - Fix README typos
See #10 - Always attach AI and Workflow cursor rules - Fix README typos
Remote ultrareview of the OAuth login flow surfaced nine issues. All non-pre-existing findings are fixed here: 1. accounts switch: persist --endpoint on the remote-account picker path so the flag is not silently dropped when selecting a remote account. 2. oauth-client: wrap requestDeviceCode and the per-poll fetch in AbortController with 15s timeouts — a hung socket used to bypass the outer deadline and spin forever. 3. accounts current: route through createControlApi so OAuth accounts get the same TokenRefreshMiddleware every other command uses (previously built ControlApi directly, skipping refresh). 4. Timer leaks: try/finally around clearTimeout in postForTokens and revokeToken; hold the revokeWithTimeout handle and clear it on settle — removes the ~5–15s process hang after logout/refresh. 5. OAuth host precedence: prefer account.controlHost over --control-host for refresh and revoke. The flag is for control-plane routing, not the OAuth issuer — a mismatch used to return invalid_grant and wipe a valid session. 6. Concurrent-refresh race: add ConfigManager.reloadConfig() and guard clearOAuthSession() with an on-disk compare. A peer that successfully rotated the refresh token no longer gets clobbered by our stale in-memory view. Includes two new middleware tests. 7. Slug collision: new pickUniqueAlias() helper auto-suffixes when a different accountId already owns the base alias. Applied to both login (JSON branch) and switch (remote-account picker); re-logging into the same account still reuses the alias. 8. login --json: warn when auto-selecting among multiple accounts, for parity with the apps/keys JSON branches. 9. config-manager: purge legacy accessToken/accessTokenExpiresAt/tokenId in storeOAuthTokens so an alias upgraded from the pre-OAuth flow does not leave a stale plaintext token at rest. Finding #10 (host.includes('local') HTTP downgrade) is pre-existing in control-api.ts and intentionally left out of scope.
1 participant
Various improvements based on existing prompts and lessons learnt building this CLI with AI agent assistance.
The improvements here should help accelerate development for the remaining tasks.
Summary by CodeRabbit