Enhance container port metadata with protocol, label, and browsable flag - #24
Conversation
…spec C.7/C.3) Adapt the SDK to GladysAssistant/Gladys#2786, which adds a `browsable` field (default true) to the manifest sub-container ports and returns the full port shape from `GET /container`: a port that serves no web UI (a WebSocket endpoint waiting for devices, the OCPP case) is shown as a plain badge instead of an "Open" link. - ContainerPort typings resynced with the core state: `protocol`, `label` and `browsable` added, `host_port` corrected to `number | null` (no host port is allocated before the first start); new exported ContainerPortProtocol union - getContainers JSDoc + README document the full port entry and what browsable: false means for the integration - tests: the getContainers fixture carries the real port shape, plus a non-browsable port with an unassigned host port; compile-time checks on the new fields Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WxpcyV2TFQvTJkYdTa9YET
|
Warning Review limit reached
Next review available in: 53 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR expands ChangesContainer port metadata
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/types/api.test-d.ts`:
- Around line 148-157: The type test currently allows host_port to be
number-only because optional chaining contributes undefined; add a direct
type-only assignment proving the underlying ContainerPort host_port accepts
null. Use the existing IntegrationContainer symbol and its ports element type,
alongside the other declarations, without changing runtime behavior.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 85af8e3f-541d-45b6-b40b-d7d4c3e5b385
📒 Files selected for processing (5)
README.mdindex.d.tslib/gladys-integration.jstest/containers.test.jstest/types/api.test-d.ts
The optional chaining on `containers[0]?.ports[0]?.host_port` contributes `undefined` on its own, so the widened `number | null | undefined` annotation type-checked even when `host_port` was `number` — the check did not actually prove the field is nullable. Assign `null` straight to `ContainerPort['host_port']` instead, which fails to compile without the null in the type. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WxpcyV2TFQvTJkYdTa9YET
Resolve the overlap with #24 (adaptation of GladysAssistant/Gladys#2786), which landed on main the port fields this branch had added on its own: keep main's version of `protocol`, `label`, `browsable` and the nullable `host_port` (its ContainerPortProtocol union and its direct-assignment type check are the stronger form), and keep only the delta specific to GladysAssistant/Gladys#2787 on top — the `name` field of ContainerPort, its JSDoc/README documentation and a dedicated test. - index.d.ts: `name: string | null` added to main's ContainerPort - getContainers JSDoc and the README port entry list `name` - README: the "Placeholders in section texts" subsection stays, the Sub-containers block explains `name` after main's `browsable` paragraph - tests: main's browsable/unassigned-port case kept as is, the port name gets its own case; the type test keeps both direct assignments Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bb3mfS9ufKovY8WxwDk9VB
Summary
This PR extends the
ContainerPortinterface to include additional metadata about published ports, enabling better representation of port capabilities in the supervision UI and providing integration developers with complete port information from the manifest.Key Changes
Extended
ContainerPortinterface with new fields:protocol: TCP or UDP protocol type (declared in manifest, defaults to TCP)label: Multi-language label for the port (from manifest)browsable: Boolean flag indicating if the port serves a web UI (defaults to true)host_portnullable (number | null) to represent unallocated portsUpdated type definitions (
index.d.ts):ContainerPortProtocoltype for protocol validationContainerPortinterface with comprehensive JSDoc commentsImproved documentation:
host_portis chosen by Gladys and persisted across recreationsbrowsableflag use case (e.g., OCPP WebSocket endpoints)Enhanced test coverage:
browsable: falseandhost_port: nullImplementation Details
host_portfield is nownullwhile Gladys has not yet allocated a port (e.g., container never started)browsableflag allows the supervision UI to distinguish between web UI ports (showing "Open" links) and non-browsable ports like WebSocket endpoints (showing plain badges)https://claude.ai/code/session_01WxpcyV2TFQvTJkYdTa9YET
Summary by CodeRabbit
New Features
Documentation
Tests