docs: initial developer guide for adding services#278
Open
rshoemaker wants to merge 6 commits intomainfrom
Open
docs: initial developer guide for adding services#278rshoemaker wants to merge 6 commits intomainfrom
rshoemaker wants to merge 6 commits intomainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new comprehensive developer doc Changes
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/development/supported-services.md (2)
55-69: Add language specification to the fenced code block.The data flow diagram should specify a language for proper rendering. Use
textif it's ASCII art, or consider usingmermaidfor a proper diagram.📝 Proposed fix
-``` +```text API Request (spec.services) → Validation (API layer)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/development/supported-services.md` around lines 55 - 69, The fenced code block containing the ASCII data flow diagram (starting with "API Request (spec.services)" and the arrows) lacks a language specifier; update the opening fence to include a language (e.g., use ```text for ASCII art or ```mermaid if converting to a mermaid diagram) so the block renders correctly, and leave the diagram contents unchanged except for the fence language token.
241-247: Add language specification to the dependency chain diagram.Similar to the data flow diagram, this code block should specify a language for proper rendering.
📝 Proposed fix
-``` +```text Phase 1: Network (swarm.network) — no dependencies ServiceUserRole (swarm.service_user_role) — no dependencies🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/development/supported-services.md` around lines 241 - 247, The fenced code block containing the dependency chain (lines showing Phase 1: Network, ServiceUserRole, ServiceInstanceSpec, ServiceInstance, ServiceInstanceMonitor) is missing a language tag and should be marked for text rendering; update the opening triple-backtick to include "text" so the block starts with ```text to ensure proper rendering of the diagram (refer to the dependency entries: Network (swarm.network), ServiceUserRole (swarm.service_user_role), ServiceInstanceSpec (swarm.service_instance_spec), ServiceInstance (swarm.service_instance), ServiceInstanceMonitor (monitor.service_instance)).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/development/supported-services.md`:
- Around line 55-69: The fenced code block containing the ASCII data flow
diagram (starting with "API Request (spec.services)" and the arrows) lacks a
language specifier; update the opening fence to include a language (e.g., use
```text for ASCII art or ```mermaid if converting to a mermaid diagram) so the
block renders correctly, and leave the diagram contents unchanged except for the
fence language token.
- Around line 241-247: The fenced code block containing the dependency chain
(lines showing Phase 1: Network, ServiceUserRole, ServiceInstanceSpec,
ServiceInstance, ServiceInstanceMonitor) is missing a language tag and should be
marked for text rendering; update the opening triple-backtick to include "text"
so the block starts with ```text to ensure proper rendering of the diagram
(refer to the dependency entries: Network (swarm.network), ServiceUserRole
(swarm.service_user_role), ServiceInstanceSpec (swarm.service_instance_spec),
ServiceInstance (swarm.service_instance), ServiceInstanceMonitor
(monitor.service_instance)).
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/development/supported-services.md`:
- Around line 55-69: The fenced flow-diagram code block is unlabeled and
triggers MD040; update the opening triple-backticks for that block to include
the language hint "text" (i.e., change ``` to ```text) so the block is
recognized as plain text and the MD040 lint warning is resolved.
- Around line 241-247: The fenced code block that lists the dependency phases
(starting with "Phase 1: Network (swarm.network)" and including "Phase 4:
ServiceInstanceMonitor (monitor.service_instance)") lacks a language hint;
update the opening fence from ``` to ```text so the block is marked as plain
text to resolve the MD040 lint warning.
- Line 1169: The cross-reference target for A.2 used in the A.7 step is
malformed (shows `apiapiapiv1validatego`); update the fragment portion of the
link that references validateMCPServiceConfig()/A.2 so it matches the actual
heading slug for A.2 (the validation function entry), replacing the incorrect
`apiapiapiv1validatego` fragment with the correct fragment that corresponds to
the A.2 heading (so the `validateMCPServiceConfig()` cross-link resolves
correctly).
- Around line 90-92: The docs currently list the fields `cpus` and `memory` with
inconsistent types across sections (overview table uses Float64/UInt64 while the
API DSL appendix marks them as String with patterns); pick the canonical type
(either numeric types Float64/UInt64 or patterned String) and make all
references consistent: update the overview table, the appendix API DSL entries,
any example payloads, and schema/pattern descriptions so `cpus` and `memory` use
the same type and validation wording throughout (ensure you update the `cpus`
and `memory` entries and any nearby examples that assume one representation).
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docs/development/supported-services.md (1)
1169-1169:⚠️ Potential issue | 🟡 MinorFix the broken cross-reference fragment for A.2.
The fragment
#a2-validation-serverinternalapiapiapiv1validategocontains a duplicateapisegment. Based on the actual heading at line 697, the correct fragment should be#a2-validation-serverinternalapiapiv1validatego.📝 Proposed fix
- `validateMCPServiceConfig()` (see [A.2](`#a2-validation-serverinternalapiapiapiv1validatego`)). + `validateMCPServiceConfig()` (see [A.2](`#a2-validation-serverinternalapiapiv1validatego`)).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/development/supported-services.md` at line 1169, Update the broken cross-reference fragment that follows `validateMCPServiceConfig()` by replacing the duplicate segment `#a2-validation-serverinternalapiapiapiv1validatego` with the correct fragment `#a2-validation-serverinternalapiapiv1validatego`; locate the occurrence near the `validateMCPServiceConfig()` reference and change only the fragment string so it matches the actual heading anchor.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@docs/development/supported-services.md`:
- Line 1169: Update the broken cross-reference fragment that follows
`validateMCPServiceConfig()` by replacing the duplicate segment
`#a2-validation-serverinternalapiapiapiv1validatego` with the correct fragment
`#a2-validation-serverinternalapiapiv1validatego`; locate the occurrence near
the `validateMCPServiceConfig()` reference and change only the fragment string
so it matches the actual heading anchor.
moizpgedge
reviewed
Feb 27, 2026
Contributor
moizpgedge
left a comment
moizpgedge
left a comment
There was a problem hiding this comment.
The docs looks good to me
tsivaprasad
reviewed
Mar 3, 2026
Contributor
tsivaprasad
left a comment
tsivaprasad
left a comment
There was a problem hiding this comment.
Looks great.
I think the document would benefit from adding a small “API Usage & Verification” section with concrete examples.
- Example POST /v1/databases request payload with a service
- Example validation error response (HTTP 400)
- Example GET /v1/databases/{id} response showing service_instances
- Example update flow (add/remove service from existing DB)
- Brief clarification on how service health/failure is reflected in API state
Add concrete request/response examples for service CRUD operations, validation errors, and health/failure state transitions. Covers create, read, update (add/remove), error responses, sensitive key stripping, and the service instance monitor lifecycle.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/development/supported-services.md`:
- Around line 153-156: Add one clear sentence in the discussed docs sections
clarifying that failures during provisioning/workflow (e.g., errors returned by
GetServiceImage when a service_type/version is not registered in the image
registry) can cause the workflow task to fail and set the database state to
"failed", whereas runtime/post-provision monitoring or health-check failures do
not automatically transition the database to "failed" and must be handled
separately by the monitoring/recovery logic; place this sentence near the
existing text that mentions service errors and DB state so readers see the
distinction (also mirror the same sentence in the other section that currently
reads inconsistently).
- Around line 257-259: The docs conflict: one section says the runtime network
name is "{databaseID}-database" while the GenerateDatabaseNetworkID(dbID)
documentation says "{dbID}"; update documentation so both use a single canonical
format or explicitly document the transformation. Edit the text describing
GenerateDatabaseNetworkID(dbID) to show the full runtime name (e.g.,
"{databaseID}-database") or add a short sentence in the earlier section
explaining that GenerateDatabaseNetworkID(dbID) returns dbID which is then
converted to the runtime name by appending "-database". Ensure both the
paragraph referencing the runtime name and the GenerateDatabaseNetworkID(dbID)
doc are consistent.
Contributor
Author
|
Thanks @tsivaprasad, I added the content you suggested. |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docs/development/supported-services.md (1)
257-262:⚠️ Potential issue | 🟡 MinorClarify identifier vs runtime network name in this paragraph.
Line 261 currently says deduplication happens because both sides generate the same
"{databaseID}-database"name, but Lines 448-451 explain matching is done via the resource identifier (GenerateDatabaseNetworkID(dbID)), with runtime Docker name set separately. Please align this paragraph to avoid implementation/test confusion.Suggested wording fix
- naturally via identifier matching (both generate the same - `"{databaseID}-database"` name). Uses the IPAM service to allocate subnets. + naturally via identifier matching (both use + `GenerateDatabaseNetworkID(dbID)` as the same resource ID). The runtime Docker + network name is `"{databaseID}-database"` (set separately on `Network.Name`). + Uses the IPAM service to allocate subnets.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/development/supported-services.md` around lines 257 - 262, The paragraph about Network deduplication is ambiguous about "name" vs the internal resource identifier; update the text to state that deduplication is done by matching the resource identifier (GenerateDatabaseNetworkID(dbID)) and that the runtime Docker network name is set separately (e.g., "{databaseID}-database"), so tests and implementation aren't confused—mention both the identifier function GenerateDatabaseNetworkID and the runtime name format to make the distinction explicit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@docs/development/supported-services.md`:
- Around line 257-262: The paragraph about Network deduplication is ambiguous
about "name" vs the internal resource identifier; update the text to state that
deduplication is done by matching the resource identifier
(GenerateDatabaseNetworkID(dbID)) and that the runtime Docker network name is
set separately (e.g., "{databaseID}-database"), so tests and implementation
aren't confused—mention both the identifier function GenerateDatabaseNetworkID
and the runtime name format to make the distinction explicit.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.gitignoredocs/development/supported-services.md
✅ Files skipped from review due to trivial changes (1)
- .gitignore
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
3 participants
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.
docs: add developer guide for adding supported services
Note: this document is a snapshot overview based on the incomplete implementation of the MCP supported service. It will need to be updated as we gain more experience adding additional services.
What's covered
PLAT-445