Comfy-Org / Comfy-Org/comfy-multi-player
Residual from #17: reject contradictory add_node identity at the wire boundary
- Dominant language
- TypeScript
- Stars
- 5
- Forks
- 1
- Avg merge
- 12h 22m
- Merged PRs (30d)
- 121
Description
At reviewed SHA `811d6a267b34f1eba2ed0969dd24ccdda565371b`, the wire boundary still accepts contradictory `add_node` identities after #17 was closed by #67 as fixed.
- `node_id: 9` with payload `node.id: 77` is accepted and stored under key `9` with payload identity `77`.
- a subsequent `connect` addressed to node `9` is also accepted.
- projection emits node `77` but a link whose destination is nonexistent projected node `9`.
Evidence, prior closure, and reproduction
Reviewed SHA: `811d6a267b34f1eba2ed0969dd24ccdda565371b`.
Exact source locations:
- validation checks only that `node_id` exists and `node` is an object: [`src/applier.ts:478-484`](https://github.com/Comfy-Org/comfy-multi-player/blob/811d6a267b34f1eba2ed0969dd24ccdda565371b/src/applier.ts#L478-L484)
- storage is keyed by `String(op.node_id)`: [`src/applier.ts:485-490`](https://github.com/Comfy-Org/comfy-multi-player/blob/811d6a267b34f1eba2ed0969dd24ccdda565371b/src/applier.ts#L485-L490)
- the payload is treated as authoritative and copied through `createNodeMap(op.node, ...)`: [`src/applier.ts:493-517`](https://github.com/Comfy-Org/comfy-multi-player/blob/811d6a267b34f1eba2ed0969dd24ccdda565371b/src/applier.ts#L493-L517)
- the resulting map is stored under the different `node_id` key: [`src/applier.ts:524-526`](https://github.com/Comfy-Org/comfy-multi-player/blob/811d6a267b34f1eba2ed0969dd24ccdda565371b/src/applier.ts#L524-L526)
- the repository's own regression demonstrates both accepted ops and the dangling projected link: [`test/invalid-op-states.test.ts:198-247`](https://github.com/Comfy-Org/comfy-multi-player/blob/811d6a267b34f1eba2ed0969dd24ccdda565371b/test/invalid-op-states.test.ts#L198-L247)
Independent verification ran the focused file with Vitest's threads pool: 13/13 tests passed, including the mismatch case. The test asserts node `77` is present, node `9` is absent, and link `500` still targets `9`.
Prior tracking: #17 named this exact `add_node` identity mismatch and asked to single-source identity. It was closed automatically by merged #67 (“make invalid op states unrepresentable”), but the current test labels the runtime wire state as accepted behavior “pinned, not endorsed.” Typed-call-site narrowing therefore did not close the JSON boundary identified in `validateEnvelope`.
Suggested root fix: reject an absent or non-equivalent payload `node.id` at the envelope/applier boundary without mutation, preserving the verbatim payload rule for valid `add_node` ops. Pin rejection byte identity and both numeric/string normalization cases.
Draft-head check: open drafts #123 (`38e6ec6fb02a68d676855dd64d271bea96edc3cd`) and #138 (`ca2a3ec25bcc0d6243a8346d1f8ed53f606cd243`) do not address this residual.
Contributor guide
Assessment
This issue has not been assessed yet.