diff --git a/loopx/control_plane/collaboration/conversation_binding.ts b/loopx/control_plane/collaboration/conversation_binding.ts index 060bf73a13..676d2c6d15 100644 --- a/loopx/control_plane/collaboration/conversation_binding.ts +++ b/loopx/control_plane/collaboration/conversation_binding.ts @@ -1,4 +1,5 @@ import type {JsonObject} from "../effect_program.ts"; +import {isDeepStrictEqual} from "node:util"; import {EffectRuntimeRequestError} from "../effect_runtime_errors.ts"; import {requireInteger, requireJsonObject, requireNonEmptyString} from "../runtime_decode.ts"; import {requireAutomationCadenceEligibility} from "../quota/automation_cadence.ts"; @@ -341,10 +342,12 @@ export function resolveBoundConversation(params: JsonObject): JsonObject { const saved = requireJsonObject(params.session_context, "bound Session context"); // New commissions may extend the same owner's scope. Workspace, role and // audience remain frozen; all evidence reads use the fresh binding scope. + // JSON object member order is not identity. Preserve every value and field + // check while accepting equivalent contexts from another JSON writer. const matches = row.context_kind === "steward" - ? JSON.stringify({...saved, goal_ids: []}) === JSON.stringify({...context, goal_ids: []}) + ? isDeepStrictEqual({...saved, goal_ids: []}, {...context, goal_ids: []}) && (row.goal_scope === "all_registered" || normalizeStewardGoalScope(saved.goal_ids).every(g => (row.goal_ids as string[]).includes(g))) - : JSON.stringify(saved) === JSON.stringify(context); + : isDeepStrictEqual(saved, context); if (!matches) throw new EffectRuntimeRequestError("bound Session context changed"); } const channel = row.context_kind === "steward" @@ -358,7 +361,7 @@ export function resolveBoundConversation(params: JsonObject): JsonObject { || session.agent_id !== row.executor_endpoint_id || session.executor_endpoint_id !== row.executor_endpoint_id || session.session_mode !== "managed_runtime" || (steward ? session.project_context : session.steward_context) != null || params.session_context === undefined - || JSON.stringify(steward ? session.steward_context : session.project_context) !== JSON.stringify(params.session_context)) { + || !isDeepStrictEqual(steward ? session.steward_context : session.project_context, params.session_context)) { throw new EffectRuntimeRequestError("canonical bound Session identity changed"); } } diff --git a/tests/control_plane_ts/conversation_binding.test.ts b/tests/control_plane_ts/conversation_binding.test.ts index 104c8754dd..b609ff34a6 100644 --- a/tests/control_plane_ts/conversation_binding.test.ts +++ b/tests/control_plane_ts/conversation_binding.test.ts @@ -142,6 +142,25 @@ for (const kind of ["project", "steward"]) { assert.throws(() => listLocalBoundSessions({...lookup, ...change, sessions: []})); } }); + + test(`${kind} bound identity is independent of JSON object property order`, () => { + const reordered = Object.fromEntries(Object.entries(context as Record).reverse()); + const field = kind === "steward" ? "steward_context" : "project_context"; + const restored = {...session, [field]: reordered}; + assert.deepEqual(listLocalBoundSessions({...lookup, sessions: [restored]}), {session_ids: ["original"]}); + // The ingress snapshot and canonical store may use different serialization + // order. Neither changes the audience, workspace or original Session. + for (const saved of [context, reordered]) { + assert.deepEqual(resolveBoundConversation({...continuation, session_context: saved, + session: restored}).context, context); + } + for (const change of [{source_ref: "f".repeat(24)}, {grant: "workspace_write"}, + {operator_ref: "f".repeat(24)}, {unexpected: true}]) { + const changed = {...reordered, ...change}; + assert.throws(() => resolveBoundConversation({...continuation, session_context: changed, + session: {...restored, [field]: changed}})); + } + }); } test("local discovery never promotes a group binding to a private recipient", () => { diff --git a/tests/test_chat_conversation_bindings.py b/tests/test_chat_conversation_bindings.py index a55c4bbf48..f3b8a54266 100644 --- a/tests/test_chat_conversation_bindings.py +++ b/tests/test_chat_conversation_bindings.py @@ -438,6 +438,43 @@ def _bound_project(fixture): return bindings, args, binding, session, turn +def test_bound_http_continuation_preserves_identity_after_json_key_reordering(ordinary): # noqa: F811 + store, runtime, _contexts, request, capture, _fake, _workspace = ordinary + bindings, _args, binding, session, active = _bound_project(ordinary) + sid = session["session_id"] + original_thread = store.load_session(sid)["upstream_thread_id"] + runtime.interrupt_turn(session_id=sid, turn_id=active["turn_id"]) + # A JSON writer may reorder object members without changing their values. + # Exercise the real store, HTTP discovery/admission and queued worker path. + saved = dict(reversed(list(session["project_context"].items()))) + try: + # Re-serialize only this idle, isolated fixture. Source context remains + # immutable through the production Session update API. + from loopx.chat_store import _atomic_write_json + _atomic_write_json(store._session_path(sid), {**store.load_session(sid), "project_context": saved}) + status, discovered = request(f"/api/chat/sessions?conversation_binding_id={binding['binding_id']}") + assert status == 200, discovered + assert [row["session_id"] for row in discovered["sessions"]] == [sid] + body = {"message": "Continue in the original project", "client_turn_id": "reordered-context", "delivery": "queue"} + status, accepted = request(f"/api/chat/sessions/{sid}/turns", body) + assert status == 202, accepted + assert runtime.wait_for_turn(session_id=sid, turn_id=accepted["turn_id"], timeout_sec=10)["status"] == "completed" + status, replay = request(f"/api/chat/sessions/{sid}/turns", body) + assert status == 202 and not replay["created"] and replay["turn_id"] == accepted["turn_id"] + current = store.load_session(sid) + assert current["goal_id"] is None and current["upstream_thread_id"] == original_thread + assert current["project_context"] == saved + inputs = [row for row in store.messages(sid) if row["turn_id"] == accepted["turn_id"] and row["role"] == "user"] + assert len(inputs) == 1 and inputs[0]["origin"] == "web" + calls = [json.loads(line) for line in capture.read_text().splitlines()] + assert sum(row.get("method") == "turn/start" for row in calls) == 2 + bindings.disconnect(binding["binding_id"], expected_revision=bindings.read()["revision"]) + status, _ = request(f"/api/chat/sessions/{sid}/turns", {**body, "client_turn_id": "revoked-context"}) + assert status == 400 and store.turn_for_client(sid, "revoked-context") is None + finally: + runtime.close() + + @pytest.mark.parametrize("committed", [False, True]) def test_http_queue_retry_retains_original_managed_acceptance(ordinary, monkeypatch, committed): # noqa: F811 store, runtime, contexts, request, capture, _fake, _workspace = ordinary