feat(websocket): complete target topology and secure release gates - #98
Merged
Merged
Conversation
Expose Home Assistant secondary entities through typed target extraction and related-registry discovery while preserving the primary-only default. Refresh audited tooling, remediate the nanoid advisory, and force authoritative PM drift validation after extension setup.
Close the implementation and security remediation items with exact coverage, live acceptance, automatic-release dry-run, and no-publish evidence. Regenerate the managed changelog from canonical PM state.
Summary by CodeRabbit
WalkthroughChangesThe CLI adds opt-in secondary-entity discovery for WebSocket Target topology and secure release
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant websocket_target as websocket-target.ts
participant HomeAssistant
CLI->>websocket_target: Run extract or related with --include-secondary
websocket_target->>HomeAssistant: Send extract_from_target with primary_entities_only:false
HomeAssistant-->>websocket_target: Return target topology
websocket_target-->>CLI: Render extraction or related results
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
Reviewer's GuideImplements optional secondary-entity inclusion for WebSocket target extraction/related helpers while keeping primary-only behavior as the default, and reworks PM/release quality gates to use the locked pm CLI with added security remediation and docs alignment. Sequence diagram for WebSocket target extract/related with optional secondary entitiessequenceDiagram
actor User
participant websocket_target_command as websocket_target_command
participant HomeAssistantWebSocket as HomeAssistantWebSocket
User->>websocket_target_command: hassio ws target extract --device-id abc123 [--include-secondary]
websocket_target_command->>websocket_target_command: toTargetPayload(TargetOptions)
websocket_target_command->>websocket_target_command: toExtractPayload(target, ExtractOptions)
alt [includeSecondary is set]
websocket_target_command->>HomeAssistantWebSocket: extract_from_target(ExtractPayload)
else [includeSecondary not set]
websocket_target_command->>HomeAssistantWebSocket: extract_from_target(ExtractPayload)
end
HomeAssistantWebSocket-->>websocket_target_command: ExtractedTarget
User->>websocket_target_command: hassio ws target related --label-id lighting [--include-secondary]
websocket_target_command->>websocket_target_command: toTargetPayload(TargetOptions)
websocket_target_command->>websocket_target_command: toExtractPayload(target, ExtractOptions)
alt [includeSecondary is set]
websocket_target_command->>HomeAssistantWebSocket: extract_from_target(ExtractPayload)
else [includeSecondary not set]
websocket_target_command->>HomeAssistantWebSocket: extract_from_target(ExtractPayload)
end
HomeAssistantWebSocket-->>websocket_target_command: ExtractedTarget
Flow diagram for updated PM quality gate using locked pm CLIflowchart LR
A[quality:pm npm script] --> B[pm --version]
B --> C[pm install npm:pm-changelog --project]
C --> D[pm merge install]
D --> E[pm gc --scope runtime --json]
E --> F[pm validate --check-files --check-resolution --check-history-drift --strict-exit --json]
F --> G[pm health --check-only --strict-exit --json]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Owner
Author
|
@greptileai please review the exact current head. |
Owner
Author
|
/gemini review |
Owner
Author
|
@coderabbitai full review |
Owner
Author
|
@sourcery-ai review |
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 @.agents/pm/history/hac-mw4f.jsonl:
- Around line 7-10: Regenerate the test-history mutations in
.agents/pm/history/hac-mw4f.jsonl lines 7-10 so every JSON Patch operation is
valid and replaying the sequence produces the canonical test order: focused
contract, coverage, then live acceptance. Synchronize
.agents/pm/tasks/hac-mw4f.toon lines 48-58 with that regenerated history output.
In @.agents/pm/history/hac-ndy5.jsonl:
- Around line 4-5: Update the linked-test normalization represented by the
history entries so reordering preserves every field from each original test
object, including timeout_seconds and note. Adjust the remove-and-add or schema
migration logic associated with metadata.tests to carry these fields into the
normalized entries, while retaining the existing command and scope values.
In `@docs/LLM_INTEGRATION.md`:
- Line 783: Update the “Resolve targets first” guidance to restrict
--include-secondary to the target extract and related subcommands, removing any
implication that it applies to triggers, conditions, or services. Keep those
subcommands listed for target resolution without the unsupported option.
In `@package.json`:
- Line 35: Update the quality:pm script so it resolves `@unbrained/pm-cli` once
per run, stores the exact resolved version, and reuses that version for merge
install, gc, validate, and health instead of independently invoking `@latest`.
Record the resolved version in the gate output while preserving the existing
command order and strict checks.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 11a56382-05ae-46a0-877f-dc28e6a28cac
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (18)
.agents/pm/features/hac-17iu.toon.agents/pm/history/hac-0t0w.jsonl.agents/pm/history/hac-17iu.jsonl.agents/pm/history/hac-mw4f.jsonl.agents/pm/history/hac-ndy5.jsonl.agents/pm/issues/hac-ndy5.toon.agents/pm/plans/hac-0t0w.toon.agents/pm/tasks/hac-mw4f.toonCHANGELOG.mdREADME.mddocs/AGENT_OPTIMIZATION.mddocs/API.mddocs/LLM_INTEGRATION.mddocs/RELEASING.mdpackage.jsonsrc/commands/websocket-target.tstests/e2e-live.tstests/websocket-command.test.ts
Reuse the single lockfile-installed pm-cli binary throughout governance and changelog gates, restore linked-test timeout and evidence metadata through PM, and clarify the subcommands that accept secondary topology.
Owner
Author
|
Bot artifact disposition for reviewed head 49eba33 and fix head 17ae6f5:
No bot artifact was treated as approval merely because its provider skipped or rate-limited. |
Owner
Author
|
@greptileai please review the exact current head 17ae6f5. |
Owner
Author
|
/gemini review |
Owner
Author
|
@coderabbitai full review |
Owner
Author
|
@sourcery-ai review |
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The
includeSecondaryhandling for bothextractandrelatedcommands duplicates the payload construction logic; consider extracting a small helper (e.g.,buildExtractPayload(target, expandGroup, includeSecondary)) to keep behavior consistent and reduce maintenance overhead. - In
toTargetPayload, the inline tuple key union type ("entityId" | "deviceId" | ...) could be replaced withkeyof TargetOptionsor a dedicated alias to avoid divergence if theTargetOptionsshape changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `includeSecondary` handling for both `extract` and `related` commands duplicates the payload construction logic; consider extracting a small helper (e.g., `buildExtractPayload(target, expandGroup, includeSecondary)`) to keep behavior consistent and reduce maintenance overhead.
- In `toTargetPayload`, the inline tuple key union type (`"entityId" | "deviceId" | ...`) could be replaced with `keyof TargetOptions` or a dedicated alias to avoid divergence if the `TargetOptions` shape changes.
## Individual Comments
### Comment 1
<location path="src/commands/websocket-target.ts" line_range="45-46" />
<code_context>
function toTargetPayload(options: TargetOptions): Record<string, string[]> {
const target: Record<string, string[]> = {};
- const mappings: Array<[keyof TargetOptions, string]> = [
+ const mappings: Array<[
+ "entityId" | "deviceId" | "areaId" | "floorId" | "labelId",
+ string,
+ ]> = [
</code_context>
<issue_to_address>
**suggestion:** Consider typing `mappings` from `keyof TargetOptions` to avoid future drift when target options change.
The explicit literal union here omits `includeSecondary` but now partially duplicates `TargetOptions`’ keys. Deriving the type instead (e.g. `Exclude<keyof TargetOptions, "includeSecondary">`) would keep this mapping aligned with `TargetOptions` as keys are added or renamed, while still excluding non-target flags. That helps avoid the list drifting out of sync and introducing subtle bugs over time.
Suggested implementation:
```typescript
floorId?: string;
labelId?: string;
includeSecondary?: boolean;
};
type TargetOptionKey = Exclude<keyof TargetOptions, "includeSecondary">;
type ExtractedTarget = {
```
```typescript
function toTargetPayload(options: TargetOptions): Record<string, string[]> {
const target: Record<string, string[]> = {};
const mappings: Array<[TargetOptionKey, string]> = [
```
1. Ensure that `TargetOptions`’ keys used in `mappings` (e.g. `entityId`, `deviceId`, `areaId`, `floorId`, `labelId`) are indeed part of `TargetOptions`; the new `TargetOptionKey` alias will otherwise cause type errors if any mapped key is not present.
2. If other non-target flags are added to `TargetOptions` in the future (similar to `includeSecondary`), they will automatically be excluded from `mappings` if you extend `TargetOptionKey`’s `Exclude` union accordingly.
</issue_to_address>
### Comment 2
<location path="tests/websocket-command.test.ts" line_range="284-294" />
<code_context>
expect(parsed.related.labels).toHaveLength(1);
});
+ it("can include secondary entities in related registry discovery", async () => {
+ call.mockResolvedValueOnce({});
+ await createWebsocketCommand().parseAsync([
+ "target", "related", "--area-id", "kitchen", "--include-secondary",
+ ], { from: "user" });
+ expect(call).toHaveBeenNthCalledWith(1, "extract_from_target", {
+ target: { area_id: ["kitchen"] },
+ expand_group: false,
+ primary_entities_only: false,
+ });
+ });
+
</code_context>
<issue_to_address>
**suggestion (testing):** Extend related-command tests to cover interaction with `--expand-group` when `--include-secondary` is also set.
Since related also supports `--expand-group`, please add a test that passes both `--expand-group` and `--include-secondary` and verifies the payload includes `expand_group: true` and `primary_entities_only: false`. This will confirm the flag wiring works correctly when both options are combined and help prevent regressions in group expansion with secondary entities.
```suggestion
it("can include secondary entities in related registry discovery", async () => {
call.mockResolvedValueOnce({});
await createWebsocketCommand().parseAsync([
"target", "related", "--area-id", "kitchen", "--include-secondary",
], { from: "user" });
expect(call).toHaveBeenNthCalledWith(1, "extract_from_target", {
target: { area_id: ["kitchen"] },
expand_group: false,
primary_entities_only: false,
});
});
it("can include secondary entities in related registry discovery when expanding groups", async () => {
call.mockResolvedValueOnce({});
await createWebsocketCommand().parseAsync([
"target", "related", "--area-id", "kitchen", "--include-secondary", "--expand-group",
], { from: "user" });
expect(call).toHaveBeenNthCalledWith(1, "extract_from_target", {
target: { area_id: ["kitchen"] },
expand_group: true,
primary_entities_only: false,
});
});
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Derive selector keys from TargetOptions, share extract_from_target payload construction across both commands, and assert combined group expansion with secondary topology. Record the exact review validation in the PM plan.
Append a canonical PM test reset and restore sequence so raw history replay and the task snapshot end in the same order while retaining timeout and evidence metadata.
Owner
Author
|
Second-loop bot disposition:
All hosted checks were green on 17ae6f5; the current head will be re-reviewed and rechecked because later review fixes were pushed. |
Owner
Author
|
@greptileai please review the exact current head 2e2535f. |
Owner
Author
|
/gemini review |
Owner
Author
|
@coderabbitai full review |
Owner
Author
|
@sourcery-ai review |
Rate Limit Exceeded
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
TargetOptionsand the newtoExtractPayloadhelper, consider explicitly documenting or constraining whenincludeSecondaryis honored (only forextract/related) to avoid accidental reuse by other target subcommands in future changes. - The PM CLI path and invocation logic is now duplicated between the
quality:pmandchangelog:pm:*scripts; consider centralizing thePM_CLI_BINusage so both paths consistently respect the same configuration and environment.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `TargetOptions` and the new `toExtractPayload` helper, consider explicitly documenting or constraining when `includeSecondary` is honored (only for `extract`/`related`) to avoid accidental reuse by other target subcommands in future changes.
- The PM CLI path and invocation logic is now duplicated between the `quality:pm` and `changelog:pm:*` scripts; consider centralizing the `PM_CLI_BIN` usage so both paths consistently respect the same configuration and environment.
## Individual Comments
### Comment 1
<location path="src/commands/websocket-target.ts" line_range="66-71" />
<code_context>
return target;
}
+function toExtractPayload(
+ target: Record<string, string[]>,
+ options: TargetOptions & { expandGroup: boolean }
+): Record<string, unknown> {
+ const payload: Record<string, unknown> = { target, expand_group: options.expandGroup };
+ if (options.includeSecondary) payload["primary_entities_only"] = false;
+ return payload;
+}
</code_context>
<issue_to_address>
**suggestion:** Make `toExtractPayload` signature and return type more specific to improve type safety.
The function only uses `expandGroup` and `includeSecondary`, so you can narrow the `options` parameter to `{ expandGroup: boolean; includeSecondary?: boolean }` and introduce a dedicated payload type such as `{ target: Record<string, string[]>; expand_group: boolean; primary_entities_only?: boolean }`. This makes the `client.call("extract_from_target", ...)` contract explicit and improves type safety if the backend payload shape changes.
Suggested implementation:
```typescript
type ExtractOptions = {
expandGroup: boolean;
includeSecondary?: boolean;
};
type ExtractPayload = {
target: Record<string, string[]>;
expand_group: boolean;
primary_entities_only?: boolean;
};
function toExtractPayload(
target: Record<string, string[]>,
options: ExtractOptions
): ExtractPayload {
const payload: ExtractPayload = {
target,
expand_group: options.expandGroup,
};
if (options.includeSecondary) {
payload.primary_entities_only = false;
}
return payload;
}
```
Anywhere `toExtractPayload` is called, you should ensure the second argument conforms to `ExtractOptions` (i.e. it has `expandGroup` and optionally `includeSecondary`). If `TargetOptions & { expandGroup: boolean }` was relied on elsewhere, you may need to refactor those call sites to construct an `ExtractOptions` object explicitly or adjust their type annotations to match the new `ExtractOptions` type.
</issue_to_address>
### Comment 2
<location path="tests/websocket-command.test.ts" line_range="203-200" />
<code_context>
});
});
+ it("includes secondary entities only when explicitly requested", async () => {
+ call.mockResolvedValueOnce({});
+ await createWebsocketCommand().parseAsync([
+ "target", "extract", "--device-id", "device-1", "--include-secondary",
+ ], { from: "user" });
+ expect(call).toHaveBeenCalledWith("extract_from_target", {
+ target: { device_id: ["device-1"] },
+ expand_group: false,
+ primary_entities_only: false,
+ });
+ });
+
</code_context>
<issue_to_address>
**suggestion (testing):** Add a complementary test for the default primary-only behavior when --include-secondary is not provided
This test confirms the opt-in path for secondary entities. To also cover the privacy-safe default, please add a matching test (e.g. `it("defaults to primary entities when --include-secondary is omitted" ...)`) that runs `hassio ws target extract --device-id device-1` without the flag and asserts the payload has the expected default for `primary_entities_only` (or omits it, per the server contract). This will guard against regressions where secondary entities are included without explicit opt-in.
Suggested implementation:
```typescript
it("includes secondary entities only when explicitly requested", async () => {
call.mockResolvedValueOnce({});
await createWebsocketCommand().parseAsync([
"target", "extract", "--device-id", "device-1", "--include-secondary",
], { from: "user" });
expect(call).toHaveBeenCalledWith("extract_from_target", {
target: { device_id: ["device-1"] },
expand_group: false,
primary_entities_only: false,
});
});
it("defaults to primary entities when --include-secondary is omitted", async () => {
call.mockResolvedValueOnce({});
await createWebsocketCommand().parseAsync([
"target", "extract", "--device-id", "device-1",
], { from: "user" });
expect(call).toHaveBeenCalledWith("extract_from_target", {
target: { device_id: ["device-1"] },
expand_group: false,
primary_entities_only: true,
});
});
```
If the server contract is to omit `primary_entities_only` when using the default behavior (instead of explicitly sending `true`), update the new test’s expectation accordingly, e.g. by removing `primary_entities_only` from the expected payload or by asserting that it is `undefined` on the argument passed to `call`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Narrow the shared builder inputs and return an explicit Home Assistant extract_from_target payload type while preserving the opt-in secondary-entity wire contract.
Owner
Author
|
Third-loop bot disposition:
Every new comment/review was reacted to and answered at its actual GitHub surface. |
Owner
Author
|
@greptileai please review the exact current head ca287b1. |
Owner
Author
|
/gemini review |
Owner
Author
|
@coderabbitai full review |
Owner
Author
|
@sourcery-ai review |
Rate Limit Exceeded
|
Owner
Author
|
Final exact-head provider disposition for ca287b1:
No actionable finding remains on the exact current head. |
Owner
Author
|
Sourcery exact-head review 4892709762 is useful confirmation that its final pass found no remaining issue; up-voted. It supersedes the earlier two-finding Sourcery loop, whose findings were already resolved or declined with exact existing-test evidence. |
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
1 participant
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.
Summary
--include-secondarysupport to WebSocket target extraction and related-registry discovery while preserving the primary-only defaultVerification
bun run release:verifyhassio ws target extract --include-secondaryacceptancebun run release:auto:dry-run: targetv2026.8.9,dry_run=true,pushed=falsePM context
Addresses #97. Upstream PM defects are tracked in unbraind/pm-cli#779 and unbraind/pm-cli#955.
No package, tag, release, or release commit was created.
Summary by Sourcery
Add optional secondary-entity support to WebSocket target topology helpers while keeping primary-only behavior by default, and restore strict PM-based release validation and security compliance.
New Features:
--include-secondaryflag onws target extractandws target relatedto include non-primary entities in target resolution when explicitly requested.Enhancements:
Build:
pmCLI for changelog generation and PM quality gates, including runtime cache GC before validation.nanoidvia overrides for security.Documentation:
--include-secondaryusage, defaults, and privacy implications across API, optimization, LLM integration, releasing guide, and README examples.Tests:
Chores:
Summary by cubic
Adds optional secondary-entity support to
ws target extractandws target relatedwhile keeping the primary-only default. Refreshes docs/governance, pinsnanoidto3.3.17, and restores secure release gates with authoritative PM validation.New Features
--include-secondarytows target extractandws target related; default stays primary-only.extractandrelatedaccept--include-secondary(aligned with Home Assistant Core 2026.8.1).Refactors
extract_from_targetpayload acrossextractandrelated, deriving selector keys from typed options and returning an explicit Home Assistant payload; sendprimary_entities_onlyonly when--include-secondaryis set, and handleexpand-groupwith secondary topology.@unbrained/pm-cliwith an authoritative rescan to keep timeout and evidence metadata stable.Written for commit ca287b1. Summary will update on new commits.