Skip to content

Commit 6ddf494

Browse files
jajajhhzclaude
andcommitted
fix: align MCP server with PR #1417 review changes
Remove Safe field from Interaction type and all test assertions. Cast ReadWriteMode to string in configmap parser. Update all test ConfigMap YAML fixtures to remove safe: fields. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 074ff0f commit 6ddf494

File tree

6 files changed

+11
-77
lines changed

6 files changed

+11
-77
lines changed

pkg/deviceapi/api_test.go

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -103,18 +103,15 @@ func newTestDeploymentAndConfigMap() []runtime.Object {
103103
"instructions": `instructions:
104104
get_temperature:
105105
readWrite: R
106-
safe: true
107106
description: |
108107
GET /get_temperature
109108
Response: {"temperature": 36.5, "unit": "celsius"}
110109
set_unit:
111110
readWrite: W
112-
safe: false
113111
description: |
114112
POST /set_unit {"unit": "fahrenheit"}
115113
status:
116114
readWrite: R
117-
safe: true
118115
`,
119116
},
120117
},
@@ -185,13 +182,11 @@ func newTestDeploymentAndConfigMap() []runtime.Object {
185182
"instructions": `instructions:
186183
move_joint:
187184
readWrite: W
188-
safe: false
189185
description: |
190186
Move a specific joint to a target angle.
191187
Topic: robot-arm/commands/move_joint
192188
joint_positions:
193189
readWrite: R
194-
safe: true
195190
description: |
196191
Real-time joint positions.
197192
Topic: robot-arm/status/joint_positions
@@ -264,15 +259,11 @@ func TestGetDeviceDesc(t *testing.T) {
264259
getTemp, ok := interactionMap["get_temperature"]
265260
require.True(t, ok, "get_temperature interaction should exist")
266261
assert.Equal(t, "R", getTemp.ReadWrite)
267-
assert.NotNil(t, getTemp.Safe)
268-
assert.True(t, *getTemp.Safe)
269262
assert.Contains(t, getTemp.Description, "GET /get_temperature")
270263

271264
setUnit, ok := interactionMap["set_unit"]
272265
require.True(t, ok, "set_unit interaction should exist")
273266
assert.Equal(t, "W", setUnit.ReadWrite)
274-
assert.NotNil(t, setUnit.Safe)
275-
assert.False(t, *setUnit.Safe)
276267
}
277268

278269
func TestGetDeviceDescNotFound(t *testing.T) {
@@ -307,14 +298,10 @@ func TestGetDeviceDescRobotArm(t *testing.T) {
307298
moveJoint, ok := interactionMap["move_joint"]
308299
require.True(t, ok)
309300
assert.Equal(t, "W", moveJoint.ReadWrite)
310-
assert.NotNil(t, moveJoint.Safe)
311-
assert.False(t, *moveJoint.Safe)
312301

313302
jointPos, ok := interactionMap["joint_positions"]
314303
require.True(t, ok)
315304
assert.Equal(t, "R", jointPos.ReadWrite)
316-
assert.NotNil(t, jointPos.Safe)
317-
assert.True(t, *jointPos.Safe)
318305
}
319306

320307
func TestListDevicesNoDevices(t *testing.T) {

pkg/deviceapi/configmap.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,7 @@ func parseInstructionsFromConfigMap(cm *corev1.ConfigMap) ([]Interaction, error)
2727
interaction := Interaction{Name: name}
2828
if instr != nil {
2929
interaction.Description = strings.TrimSpace(instr.Description)
30-
interaction.ReadWrite = instr.ReadWrite
31-
interaction.Safe = instr.Safe
30+
interaction.ReadWrite = string(instr.ReadWrite)
3231
}
3332
interactions = append(interactions, interaction)
3433
}

pkg/deviceapi/resolver_test.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,11 @@ func TestParseInstructionsFromConfigMap(t *testing.T) {
1919
"instructions": `instructions:
2020
read_temp:
2121
readWrite: R
22-
safe: true
2322
description: |
2423
GET /read_temp
2524
Returns temperature in Celsius.
2625
write_config:
2726
readWrite: W
28-
safe: false
2927
description: |
3028
POST /write_config with JSON body.
3129
legacy_instruction:
@@ -44,19 +42,14 @@ func TestParseInstructionsFromConfigMap(t *testing.T) {
4442

4543
readTemp := interactionMap["read_temp"]
4644
assert.Equal(t, "R", readTemp.ReadWrite)
47-
assert.NotNil(t, readTemp.Safe)
48-
assert.True(t, *readTemp.Safe)
4945
assert.Contains(t, readTemp.Description, "GET /read_temp")
5046

5147
writeConfig := interactionMap["write_config"]
5248
assert.Equal(t, "W", writeConfig.ReadWrite)
53-
assert.NotNil(t, writeConfig.Safe)
54-
assert.False(t, *writeConfig.Safe)
5549

5650
// Legacy instruction without extended fields.
5751
legacy := interactionMap["legacy_instruction"]
5852
assert.Equal(t, "", legacy.ReadWrite)
59-
assert.Nil(t, legacy.Safe)
6053
assert.Equal(t, "", legacy.Description)
6154
}
6255

pkg/deviceapi/types.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,5 @@ type DeviceDesc struct {
2525
type Interaction struct {
2626
Name string `json:"name"`
2727
ReadWrite string `json:"readWrite,omitempty"`
28-
Safe *bool `json:"safe,omitempty"`
2928
Description string `json:"description,omitempty"`
3029
}

pkg/mcp/server/e2e_spec_test.go

Lines changed: 10 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,6 @@ func specK8sObjects() []runtime.Object {
172172
"instructions": `instructions:
173173
move_joint:
174174
readWrite: W
175-
safe: false
176175
description: |
177176
Move a specific joint to a target angle.
178177
## Topic
@@ -186,7 +185,6 @@ func specK8sObjects() []runtime.Object {
186185
- ` + "`speed`" + `: 1-100 (% of max speed)
187186
gripper:
188187
readWrite: W
189-
safe: false
190188
description: |
191189
Open or close the gripper.
192190
## Topic
@@ -197,15 +195,13 @@ func specK8sObjects() []runtime.Object {
197195
` + "```" + `
198196
joint_positions:
199197
readWrite: R
200-
safe: true
201198
description: |
202199
Real-time joint positions. Subscribe to receive continuous updates.
203200
## Topic
204201
` + "`robot-arm/status/joint_positions`" + `
205202
Published every 100ms. Array is [J1..J6] in degrees.
206203
emergency_stop:
207204
readWrite: W
208-
safe: false
209205
description: |
210206
Immediately halt all motion.
211207
## Topic
@@ -261,20 +257,17 @@ func specK8sObjects() []runtime.Object {
261257
"instructions": `instructions:
262258
temperature:
263259
readWrite: R
264-
safe: true
265260
description: |
266261
Temperature readings. Subject: ` + "`sensors.<node_id>.temperature`" + `
267262
Wildcard: ` + "`sensors.*.temperature`" + ` for all nodes.
268263
Published every 5 seconds per node.
269264
vibration:
270265
readWrite: R
271-
safe: true
272266
description: |
273267
Vibration readings. Subject: ` + "`sensors.<node_id>.vibration`" + `
274268
Values above 0.5g indicate potential failure.
275269
configure_interval:
276270
readWrite: W
277-
safe: false
278271
description: |
279272
Change reporting interval. Uses NATS request/reply.
280273
Subject: ` + "`sensors.<node_id>.config.interval`" + `
@@ -329,20 +322,17 @@ func specK8sObjects() []runtime.Object {
329322
"instructions": `instructions:
330323
get_temperature:
331324
readWrite: R
332-
safe: true
333325
description: |
334326
GET /get_temperature
335327
Response: {"temperature": 36.5, "unit": "celsius"}
336328
Updates every 3 seconds.
337329
set_unit:
338330
readWrite: W
339-
safe: false
340331
description: |
341332
POST /set_unit {"unit": "fahrenheit"}
342333
Response: {"status": "ok", "unit": "fahrenheit"}
343334
status:
344335
readWrite: R
345-
safe: true
346336
description: |
347337
GET /status — returns plain text: ` + "`running`" + ` or ` + "`error: <message>`" + `.
348338
`,
@@ -466,34 +456,26 @@ func TestE2ESpecRobotArmMQTT(t *testing.T) {
466456
interactionMap[intr.Name] = intr
467457
}
468458

469-
// move_joint: W, unsafe, topic info
459+
// move_joint: W, topic info
470460
moveJoint := interactionMap["move_joint"]
471461
assert.Equal(t, "W", moveJoint.ReadWrite)
472-
require.NotNil(t, moveJoint.Safe)
473-
assert.False(t, *moveJoint.Safe)
474462
assert.Contains(t, moveJoint.Description, "robot-arm/commands/move_joint")
475463
assert.Contains(t, moveJoint.Description, "joint")
476464
assert.Contains(t, moveJoint.Description, "angle")
477465

478-
// gripper: W, unsafe
466+
// gripper: W
479467
gripper := interactionMap["gripper"]
480468
assert.Equal(t, "W", gripper.ReadWrite)
481-
require.NotNil(t, gripper.Safe)
482-
assert.False(t, *gripper.Safe)
483469
assert.Contains(t, gripper.Description, "robot-arm/commands/gripper")
484470

485-
// joint_positions: R, safe
471+
// joint_positions: R
486472
jointPos := interactionMap["joint_positions"]
487473
assert.Equal(t, "R", jointPos.ReadWrite)
488-
require.NotNil(t, jointPos.Safe)
489-
assert.True(t, *jointPos.Safe)
490474
assert.Contains(t, jointPos.Description, "robot-arm/status/joint_positions")
491475

492-
// emergency_stop: W, unsafe
476+
// emergency_stop: W
493477
eStop := interactionMap["emergency_stop"]
494478
assert.Equal(t, "W", eStop.ReadWrite)
495-
require.NotNil(t, eStop.Safe)
496-
assert.False(t, *eStop.Safe)
497479
assert.Contains(t, eStop.Description, "robot-arm/commands/emergency_stop")
498480
}
499481

@@ -536,26 +518,20 @@ func TestE2ESpecSensorArrayNATS(t *testing.T) {
536518
interactionMap[intr.Name] = intr
537519
}
538520

539-
// temperature: R, safe, NATS subject pattern
521+
// temperature: R, NATS subject pattern
540522
temp := interactionMap["temperature"]
541523
assert.Equal(t, "R", temp.ReadWrite)
542-
require.NotNil(t, temp.Safe)
543-
assert.True(t, *temp.Safe)
544524
assert.Contains(t, temp.Description, "sensors.")
545525
assert.Contains(t, temp.Description, "temperature")
546526

547-
// vibration: R, safe
527+
// vibration: R
548528
vib := interactionMap["vibration"]
549529
assert.Equal(t, "R", vib.ReadWrite)
550-
require.NotNil(t, vib.Safe)
551-
assert.True(t, *vib.Safe)
552530
assert.Contains(t, vib.Description, "vibration")
553531

554-
// configure_interval: W, unsafe, request/reply
532+
// configure_interval: W, request/reply
555533
cfgInterval := interactionMap["configure_interval"]
556534
assert.Equal(t, "W", cfgInterval.ReadWrite)
557-
require.NotNil(t, cfgInterval.Safe)
558-
assert.False(t, *cfgInterval.Safe)
559535
assert.Contains(t, cfgInterval.Description, "request/reply")
560536
}
561537

@@ -593,25 +569,19 @@ func TestE2ESpecThermometerHTTP(t *testing.T) {
593569
interactionMap[intr.Name] = intr
594570
}
595571

596-
// get_temperature: R, safe, GET endpoint
572+
// get_temperature: R, GET endpoint
597573
getTemp := interactionMap["get_temperature"]
598574
assert.Equal(t, "R", getTemp.ReadWrite)
599-
require.NotNil(t, getTemp.Safe)
600-
assert.True(t, *getTemp.Safe)
601575
assert.Contains(t, getTemp.Description, "GET /get_temperature")
602576

603-
// set_unit: W, unsafe, POST endpoint
577+
// set_unit: W, POST endpoint
604578
setUnit := interactionMap["set_unit"]
605579
assert.Equal(t, "W", setUnit.ReadWrite)
606-
require.NotNil(t, setUnit.Safe)
607-
assert.False(t, *setUnit.Safe)
608580
assert.Contains(t, setUnit.Description, "POST /set_unit")
609581

610-
// status: R, safe
582+
// status: R
611583
status := interactionMap["status"]
612584
assert.Equal(t, "R", status.ReadWrite)
613-
require.NotNil(t, status.Safe)
614-
assert.True(t, *status.Safe)
615585
assert.Contains(t, status.Description, "GET /status")
616586
}
617587

@@ -760,7 +730,6 @@ func TestE2ESpecGracefulDegradation(t *testing.T) {
760730
for _, intr := range desc.Interactions {
761731
interactionNames[intr.Name] = true
762732
assert.Equal(t, "", intr.ReadWrite, "legacy instructions have no readWrite")
763-
assert.Nil(t, intr.Safe, "legacy instructions have no safe field")
764733
assert.Equal(t, "", intr.Description, "legacy instructions have no description")
765734
}
766735
assert.True(t, interactionNames["read_value"])

pkg/mcp/server/server_test.go

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -110,13 +110,11 @@ func testK8sObjects() []runtime.Object {
110110
"instructions": `instructions:
111111
get_temperature:
112112
readWrite: R
113-
safe: true
114113
description: |
115114
GET /get_temperature
116115
Response: {"temperature": 36.5, "unit": "celsius"}
117116
set_unit:
118117
readWrite: W
119-
safe: false
120118
description: |
121119
POST /set_unit {"unit": "fahrenheit"}
122120
`,
@@ -175,20 +173,17 @@ func testK8sObjects() []runtime.Object {
175173
"instructions": `instructions:
176174
move_joint:
177175
readWrite: W
178-
safe: false
179176
description: |
180177
Move a specific joint to a target angle.
181178
Topic: robot-arm/commands/move_joint
182179
Message format: {"joint": 1, "angle": 45.0, "speed": 50}
183180
joint_positions:
184181
readWrite: R
185-
safe: true
186182
description: |
187183
Real-time joint positions. Subscribe to receive continuous updates.
188184
Topic: robot-arm/status/joint_positions
189185
emergency_stop:
190186
readWrite: W
191-
safe: false
192187
description: |
193188
Immediately halt all motion. Publish any message to trigger.
194189
Topic: robot-arm/commands/emergency_stop
@@ -308,15 +303,11 @@ func TestMCPGetDeviceDescHTTP(t *testing.T) {
308303
getTemp, ok := interactionMap["get_temperature"]
309304
require.True(t, ok)
310305
assert.Equal(t, "R", getTemp.ReadWrite)
311-
assert.NotNil(t, getTemp.Safe)
312-
assert.True(t, *getTemp.Safe)
313306
assert.Contains(t, getTemp.Description, "GET /get_temperature")
314307

315308
setUnit, ok := interactionMap["set_unit"]
316309
require.True(t, ok)
317310
assert.Equal(t, "W", setUnit.ReadWrite)
318-
assert.NotNil(t, setUnit.Safe)
319-
assert.False(t, *setUnit.Safe)
320311
}
321312

322313
func TestMCPGetDeviceDescMQTT(t *testing.T) {
@@ -346,15 +337,11 @@ func TestMCPGetDeviceDescMQTT(t *testing.T) {
346337
moveJoint, ok := interactionMap["move_joint"]
347338
require.True(t, ok)
348339
assert.Equal(t, "W", moveJoint.ReadWrite)
349-
assert.NotNil(t, moveJoint.Safe)
350-
assert.False(t, *moveJoint.Safe)
351340
assert.Contains(t, moveJoint.Description, "robot-arm/commands/move_joint")
352341

353342
jointPos, ok := interactionMap["joint_positions"]
354343
require.True(t, ok)
355344
assert.Equal(t, "R", jointPos.ReadWrite)
356-
assert.NotNil(t, jointPos.Safe)
357-
assert.True(t, *jointPos.Safe)
358345

359346
eStop, ok := interactionMap["emergency_stop"]
360347
require.True(t, ok)

0 commit comments

Comments
 (0)