feat(feedback): implement feedback-o-tron invoke dispatch (FFI-backed) (#193)

hyperpolymath · claude · web-flow · commit 3f49db3f45bf · 2026-06-04T13:40:52.000+01:00
## What Builds feedback-o-tron *for real* (the follow-up you green-lit), replacing the "Grade D Alpha" stub in `feedback_ffi.zig`'s `boj_cartridge_invoke` with a live, state-machine-backed implementation, and switching the e2e cycle to the cartridge's **actual declared tools**. ## Key insight The stub already dispatched the **correct** `cartridge.json` tool names (`feedback_register_channel/start_collecting/submit/get_stats`) — it just returned `{"status":"stub"}`. The old e2e cycle invoked tools that **never existed** (`open_channel/summary/export/status/list_channels`), which is why it got `{}`. So the fix is: make the FFI real, and point the test at the real tools. ## Changes - **`feedback_ffi.zig`** — `boj_cartridge_invoke` now backs the 4 manifest tools with the existing `fb_*` channel state machine: - `feedback_register_channel` → `fb_register` → `{"slot":N,"channel":C,"state":1}` - `feedback_start_collecting` → `fb_start_collecting` → `{"slot":N,"collecting":true,"state":2}` - `feedback_submit` → `fb_submit` → `{"recorded":true,"slot":N,"feedback_count":K}` - `feedback_get_stats` → `fb_count`/`fb_positive_count`/… → `{"total_feedback":…,"positive":…,…}` - `channel`/`slot`/`sentiment` are read from the JSON args via a small, `std.json`-free byte scanner (`jsonInt`) — sufficient for the flat arg objects and version-stable. - **`feedback_ffi.zig` test** — the in-file invoke test now drives the real `register → start → submit ×2 → get_stats` cycle and asserts the real shapes (so `zig build test` covers it in CI). - **`tests/e2e_full.sh` Step 6** — rewritten to exercise the real tools via the `"arguments"` object (the field BojRest forwards to the FFI); asserts `slot` / `collecting:true` / `recorded:true` / `total_feedback`. Removes the `FEEDBACK_OTRON` skip from #192. - **No Elixir change** — the router already forwards `arguments` and returns the FFI's parsed JSON as the body. The cross-language flow (Zig JSON → Invoker parse → Jason re-encode → test) lands the keys the assertions read. ## Verification — please note `e2e_full.sh` passes `bash -n` + `shellcheck` locally. **The Zig could not be compiled locally**: this sandbox's network is GitHub-only — `ziglang.org` returns **403** and Zig isn't on GitHub releases, and apt is blocked. So the FFI is verified by **CI** (`abi-drift.yml` + `e2e.yml` both run `zig build`) and the added Zig unit test. I'll watch this PR and fix any compile error promptly. (If you'd rather I verify locally, allowing `ziglang.org` in the environment's network policy would let me install Zig 0.15.1.) The ABI is unchanged (only the dispatch body), so `abi-drift` should stay green. 🤖 Draft via Claude Code. --- _Generated by [Claude Code](https://claude.ai/code/session_019tMcRS1Dm1nWjjYP4WvbJa)_ Co-authored-by: Claude <noreply@anthropic.com>
diff --git a/cartridges/feedback-mcp/ffi/feedback_ffi.zig b/cartridges/feedback-mcp/ffi/feedback_ffi.zig
@@ -259,29 +259,69 @@ pub export fn boj_cartridge_version() [*:0]const u8 {
 
 const shim = @import("cartridge_shim.zig");
 
-/// Dispatch the cartridge.json MCP tools. Grade D Alpha — each arm
-/// returns a stub JSON body shaped to the tool's intended response.
+/// Extract a (possibly negative) integer value for a quoted JSON key from a
+/// flat object string — e.g. `jsonInt(args, "\"slot\"", 0)`. A deliberate
+/// plain byte-scan (no std.json dependency) sufficient for the cartridge's
+/// flat `{ "slot": N, "sentiment": N }` argument objects. Returns `default`
+/// when the key or its number is absent.
+fn jsonInt(json: [*c]const u8, quoted_key: []const u8, default: c_int) c_int {
+    if (json == null) return default;
+    const s = std.mem.span(@as([*:0]const u8, @ptrCast(json)));
+    const kpos = std.mem.indexOf(u8, s, quoted_key) orelse return default;
+    var i = kpos + quoted_key.len;
+    while (i < s.len and (s[i] == ' ' or s[i] == ':')) : (i += 1) {}
+    var neg: c_int = 1;
+    if (i < s.len and s[i] == '-') {
+        neg = -1;
+        i += 1;
+    }
+    var val: c_int = 0;
+    var found = false;
+    while (i < s.len and s[i] >= '0' and s[i] <= '9') : (i += 1) {
+        val = val * 10 + @as(c_int, @intCast(s[i] - '0'));
+        found = true;
+    }
+    return if (found) neg * val else default;
+}
+
+/// Dispatch the cartridge.json MCP tools against the live feedback state
+/// machine (the `fb_*` exports above). Reads `channel`/`slot`/`sentiment`
+/// from the JSON args object and returns each tool's real result shape.
 export fn boj_cartridge_invoke(
     tool_name: [*c]const u8,
     json_args: [*c]const u8,
     out_buf: [*c]u8,
     in_out_len: [*c]usize,
 ) callconv(.c) i32 {
-    _ = json_args;
     if (shim.invokeArgsNull(tool_name, out_buf, in_out_len)) return shim.RC_BAD_ARGS;
 
-    const body: []const u8 =     if (shim.toolIs(tool_name, "feedback_register_channel"))
-        "{\"result\":{\"status\":\"stub\"}}"
-    else if (shim.toolIs(tool_name, "feedback_start_collecting"))
-        "{\"result\":{\"status\":\"stub\"}}"
-    else if (shim.toolIs(tool_name, "feedback_submit"))
-        "{\"result\":{\"status\":\"stub\"}}"
-    else if (shim.toolIs(tool_name, "feedback_get_stats"))
-        "{\"result\":{\"metadata\":{},\"status\":\"stub\"}}"
-else
-    return shim.RC_UNKNOWN_TOOL;
-
-    return shim.writeResult(out_buf, in_out_len, body);
+    var buf: [512]u8 = undefined;
+
+    if (shim.toolIs(tool_name, "feedback_register_channel")) {
+        const ch = jsonInt(json_args, "\"channel\"", @intFromEnum(FeedbackChannel.api_endpoint));
+        const slot = fb_register(ch);
+        if (slot < 0) return shim.writeResult(out_buf, in_out_len, "{\"error\":\"no_channel_slots_available\"}");
+        const body = std.fmt.bufPrint(&buf, "{{\"slot\":{d},\"channel\":{d},\"state\":{d}}}", .{ slot, ch, @intFromEnum(FeedbackState.channel_registered) }) catch return shim.RC_RUNTIME_ERROR;
+        return shim.writeResult(out_buf, in_out_len, body);
+    } else if (shim.toolIs(tool_name, "feedback_start_collecting")) {
+        const slot = jsonInt(json_args, "\"slot\"", 0);
+        const rc = fb_start_collecting(slot);
+        const body = std.fmt.bufPrint(&buf, "{{\"slot\":{d},\"collecting\":{},\"state\":{d}}}", .{ slot, rc == 0, fb_state(slot) }) catch return shim.RC_RUNTIME_ERROR;
+        return shim.writeResult(out_buf, in_out_len, body);
+    } else if (shim.toolIs(tool_name, "feedback_submit")) {
+        const slot = jsonInt(json_args, "\"slot\"", 0);
+        const sentiment = jsonInt(json_args, "\"sentiment\"", @intFromEnum(Sentiment.neutral));
+        const count = fb_submit(slot, sentiment);
+        if (count < 0) return shim.writeResult(out_buf, in_out_len, "{\"recorded\":false,\"error\":\"channel_not_collecting\"}");
+        const body = std.fmt.bufPrint(&buf, "{{\"recorded\":true,\"slot\":{d},\"feedback_count\":{d}}}", .{ slot, count }) catch return shim.RC_RUNTIME_ERROR;
+        return shim.writeResult(out_buf, in_out_len, body);
+    } else if (shim.toolIs(tool_name, "feedback_get_stats")) {
+        const slot = jsonInt(json_args, "\"slot\"", 0);
+        const body = std.fmt.bufPrint(&buf, "{{\"slot\":{d},\"total_feedback\":{d},\"positive\":{d},\"negative\":{d},\"neutral\":{d},\"state\":{d}}}", .{ slot, fb_count(slot), fb_positive_count(slot), fb_negative_count(slot), fb_neutral_count(slot), fb_state(slot) }) catch return shim.RC_RUNTIME_ERROR;
+        return shim.writeResult(out_buf, in_out_len, body);
+    } else {
+        return shim.RC_UNKNOWN_TOOL;
+    }
 }
 
 // ═══════════════════════════════════════════════════════════════════════
@@ -344,20 +384,29 @@ test "state transition validation" {
 // ADR-0006 invoke dispatch tests
 // ═══════════════════════════════════════════════════════════════════════
 
-test "invoke: each declared tool succeeds" {
-    var buf: [256]u8 = undefined;
-    const tools = [_][]const u8{
-        "feedback_register_channel",
-        "feedback_start_collecting",
-        "feedback_submit",
-        "feedback_get_stats",
-    };
-    for (tools) |t| {
-        var len: usize = buf.len;
-        const rc = boj_cartridge_invoke(t.ptr, "{}", &buf, &len);
-        try std.testing.expectEqual(@as(i32, 0), rc);
-        try std.testing.expect(std.mem.indexOf(u8, buf[0..len], "result") != null);
-    }
+test "invoke: register -> start -> submit -> stats cycle returns real shapes" {
+    fb_reset();
+    var buf: [512]u8 = undefined;
+
+    var len: usize = buf.len;
+    try std.testing.expectEqual(@as(i32, 0), boj_cartridge_invoke("feedback_register_channel", "{\"channel\":2}", &buf, &len));
+    try std.testing.expect(std.mem.indexOf(u8, buf[0..len], "\"slot\":0") != null);
+
+    len = buf.len;
+    try std.testing.expectEqual(@as(i32, 0), boj_cartridge_invoke("feedback_start_collecting", "{\"slot\":0}", &buf, &len));
+    try std.testing.expect(std.mem.indexOf(u8, buf[0..len], "\"collecting\":true") != null);
+
+    len = buf.len;
+    try std.testing.expectEqual(@as(i32, 0), boj_cartridge_invoke("feedback_submit", "{\"slot\":0,\"sentiment\":1}", &buf, &len));
+    try std.testing.expect(std.mem.indexOf(u8, buf[0..len], "\"recorded\":true") != null);
+
+    len = buf.len;
+    try std.testing.expectEqual(@as(i32, 0), boj_cartridge_invoke("feedback_submit", "{\"slot\":0,\"sentiment\":-1}", &buf, &len));
+    try std.testing.expect(std.mem.indexOf(u8, buf[0..len], "\"recorded\":true") != null);
+
+    len = buf.len;
+    try std.testing.expectEqual(@as(i32, 0), boj_cartridge_invoke("feedback_get_stats", "{\"slot\":0}", &buf, &len));
+    try std.testing.expect(std.mem.indexOf(u8, buf[0..len], "\"total_feedback\":2") != null);
 }
 
 test "invoke: unknown tool returns -1" {
diff --git a/tests/e2e_full.sh b/tests/e2e_full.sh
@@ -222,64 +222,43 @@ echo ""
 # POST /cartridge/:name/invoke endpoint (singular `cartridge`).
 bold "Step 6: Feedback-o-tron full cycle"
 
-# feedback-o-tron's cartridge FFI (cartridges/feedback-mcp/ffi/feedback_ffi.zig,
-# boj_cartridge_invoke) is a self-described "Grade D Alpha" stub: it answers only
-# feedback_register_channel/start_collecting/submit/get_stats with a stub body and
-# returns RC_UNKNOWN_TOOL for everything else. The full open_channel/submit/
-# summary/export/status/list_channels cycle below is not yet implemented, so we
-# record it as skipped rather than assert a feature that does not exist. Set
-# FEEDBACK_OTRON=1 once the FFI (or a native Elixir handler) implements the cycle.
-if [ "${FEEDBACK_OTRON:-0}" != "1" ]; then
-  yellow "  SKIP: feedback-o-tron full cycle — cartridge FFI is a Grade-D-Alpha stub (not yet implemented)"
-  SKIP=$((SKIP + 7))
-else
-# 6a: Register (open_channel)
-reg_result=$(curl -sf -X POST "$BASE_URL/cartridge/feedback-mcp/invoke" \
-    -H "Content-Type: application/json" \
-    -d '{"tool":"open_channel","args":"{\"channel\":\"api\"}"}' 2>/dev/null || echo "{}")
-check "feedback register (open_channel)" '"slot"' "$reg_result"
-FEEDBACK_SLOT=$(echo "$reg_result" | jq -r '.slot // "0"' 2>/dev/null || echo "0")
-
-# 6b: Submit feedback (positive)
-submit_result=$(curl -sf -X POST "$BASE_URL/cartridge/feedback-mcp/invoke" \
-    -H "Content-Type: application/json" \
-    -d "{\"tool\":\"submit\",\"args\":\"{\\\"slot\\\":\\\"${FEEDBACK_SLOT}\\\",\\\"sentiment\\\":\\\"positive\\\"}\"}" \
-    2>/dev/null || echo "{}")
-check "feedback submit (positive)" '"recorded"' "$submit_result"
+# All dispatches go through the unified POST /cartridge/:name/invoke endpoint.
+# The feedback-mcp FFI (feedback_ffi.zig boj_cartridge_invoke) backs the
+# cartridge.json tools with the live channel state machine; arguments are passed
+# as a JSON object under "arguments" (the field BojRest forwards to the FFI).
 
-# 6c: Submit feedback (negative)
-submit_neg=$(curl -sf -X POST "$BASE_URL/cartridge/feedback-mcp/invoke" \
+# 6a: Register an api_endpoint channel (FeedbackChannel.api_endpoint = 2)
+reg_result=$(curl -sf -X POST "$BASE_URL/cartridge/feedback-mcp/invoke" \
     -H "Content-Type: application/json" \
-    -d "{\"tool\":\"submit\",\"args\":\"{\\\"slot\\\":\\\"${FEEDBACK_SLOT}\\\",\\\"sentiment\\\":\\\"negative\\\"}\"}" \
-    2>/dev/null || echo "{}")
-check "feedback submit (negative)" '"recorded"' "$submit_neg"
+    -d '{"tool":"feedback_register_channel","arguments":{"channel":2}}' 2>/dev/null || echo "{}")
+check "feedback register channel" '"slot"' "$reg_result"
+FEEDBACK_SLOT=$(echo "$reg_result" | jq -r '.slot // 0' 2>/dev/null || echo 0)
 
-# 6d: Summary
-summary_result=$(curl -sf -X POST "$BASE_URL/cartridge/feedback-mcp/invoke" \
+# 6b: Start collecting on the slot
+collect_result=$(curl -sf -X POST "$BASE_URL/cartridge/feedback-mcp/invoke" \
     -H "Content-Type: application/json" \
-    -d '{"tool":"summary","args":"{}"}' 2>/dev/null || echo "{}")
-check "feedback summary" '"total_feedback"' "$summary_result"
+    -d "{\"tool\":\"feedback_start_collecting\",\"arguments\":{\"slot\":${FEEDBACK_SLOT}}}" 2>/dev/null || echo "{}")
+check "feedback start collecting" '"collecting":true' "$collect_result"
 
-# 6e: Export
-export_result=$(curl -sf -X POST "$BASE_URL/cartridge/feedback-mcp/invoke" \
+# 6c: Submit positive (sentiment: positive = 1)
+submit_pos=$(curl -sf -X POST "$BASE_URL/cartridge/feedback-mcp/invoke" \
     -H "Content-Type: application/json" \
-    -d '{"tool":"export_feedback","args":"{}"}' 2>/dev/null || echo "{}")
-check "feedback export" '"export_feedback"' "$export_result"
+    -d "{\"tool\":\"feedback_submit\",\"arguments\":{\"slot\":${FEEDBACK_SLOT},\"sentiment\":1}}" 2>/dev/null || echo "{}")
+check "feedback submit (positive)" '"recorded":true' "$submit_pos"
 
-# 6f: Status check
-status_result=$(curl -sf -X POST "$BASE_URL/cartridge/feedback-mcp/invoke" \
+# 6d: Submit negative (sentiment: negative = -1)
+submit_neg=$(curl -sf -X POST "$BASE_URL/cartridge/feedback-mcp/invoke" \
     -H "Content-Type: application/json" \
-    -d '{"tool":"status","args":"{}"}' 2>/dev/null || echo "{}")
-check "feedback status" '"feedback-o-tron"' "$status_result"
+    -d "{\"tool\":\"feedback_submit\",\"arguments\":{\"slot\":${FEEDBACK_SLOT},\"sentiment\":-1}}" 2>/dev/null || echo "{}")
+check "feedback submit (negative)" '"recorded":true' "$submit_neg"
 
-# 6g: Deregister — close the channel
-# Note: deregister is done via the FFI fb_deregister; we test through
-# the list_channels tool which should still work after channel close
-list_result=$(curl -sf -X POST "$BASE_URL/cartridge/feedback-mcp/invoke" \
+# 6e: Stats — expect the two submissions to be counted
+stats_result=$(curl -sf -X POST "$BASE_URL/cartridge/feedback-mcp/invoke" \
     -H "Content-Type: application/json" \
-    -d '{"tool":"list_channels","args":"{}"}' 2>/dev/null || echo "{}")
-check "feedback list_channels" '"available"' "$list_result"
-fi
+    -d "{\"tool\":\"feedback_get_stats\",\"arguments\":{\"slot\":${FEEDBACK_SLOT}}}" 2>/dev/null || echo "{}")
+check "feedback get_stats" '"total_feedback"' "$stats_result"
+fb_total=$(echo "$stats_result" | jq -r '.total_feedback // 0' 2>/dev/null || echo 0)
+check "feedback recorded 2 submissions" "1" "$([ "$fb_total" -ge 2 ] && echo 1 || echo 0)"
 echo ""
 
 # Step 7 (order-ticket flow) lives in tests/order_ticket_e2e.sh against
