-
Notifications
You must be signed in to change notification settings - Fork 0
Phase 3: Add web-test CI job, harden security gates, add P0 tests #221
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: phase-2/critical-bugs
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| """Tests for zero_prompt/event_bus.py — event fan-out to SSE clients.""" | ||
|
|
||
| import asyncio | ||
|
|
||
| import pytest | ||
|
|
||
| from agent.zero_prompt.event_bus import ( | ||
| _client_sessions, | ||
| _event_queues, | ||
| push_zp_event, | ||
| register_zp_client, | ||
| unregister_zp_client, | ||
| ) | ||
|
|
||
|
|
||
| @pytest.fixture(autouse=True) | ||
| def _clean_bus(): | ||
| _event_queues.clear() | ||
| _client_sessions.clear() | ||
| yield | ||
| _event_queues.clear() | ||
| _client_sessions.clear() | ||
|
|
||
|
|
||
| class TestRegisterUnregister: | ||
| def test_register_creates_queue(self): | ||
| q = register_zp_client("c1", "s1") | ||
| assert isinstance(q, asyncio.Queue) | ||
| assert "c1" in _event_queues | ||
|
|
||
| def test_unregister_removes_client(self): | ||
| register_zp_client("c1", "s1") | ||
| unregister_zp_client("c1") | ||
| assert "c1" not in _event_queues | ||
| assert "c1" not in _client_sessions | ||
|
|
||
| def test_unregister_nonexistent_is_safe(self): | ||
| unregister_zp_client("nonexistent") | ||
|
|
||
|
|
||
| class TestPushEvent: | ||
| def test_event_reaches_subscribed_client(self): | ||
| q = register_zp_client("c1", "s1") | ||
| push_zp_event({"session_id": "s1", "type": "card.update"}) | ||
| assert q.qsize() == 1 | ||
| event = q.get_nowait() | ||
| assert event["type"] == "card.update" | ||
|
|
||
| def test_event_skips_different_session(self): | ||
| q = register_zp_client("c1", "s1") | ||
| push_zp_event({"session_id": "s2", "type": "card.update"}) | ||
| assert q.qsize() == 0 | ||
|
|
||
| def test_event_without_session_reaches_all(self): | ||
| q1 = register_zp_client("c1", "s1") | ||
| q2 = register_zp_client("c2", "s2") | ||
| push_zp_event({"type": "global_event"}) | ||
| assert q1.qsize() == 1 | ||
| assert q2.qsize() == 1 | ||
|
|
||
| def test_client_without_session_receives_all(self): | ||
| q = register_zp_client("c1", None) | ||
| push_zp_event({"session_id": "any-session", "type": "card.update"}) | ||
| assert q.qsize() == 1 | ||
|
|
||
| def test_queue_full_event_dropped_silently(self): | ||
| q = register_zp_client("c1", "s1") | ||
| # Fill the queue to max | ||
| for i in range(300): | ||
| push_zp_event({"session_id": "s1", "type": f"event_{i}"}) | ||
| assert q.qsize() == 300 | ||
| # This should not raise | ||
| push_zp_event({"session_id": "s1", "type": "overflow"}) | ||
| assert q.qsize() == 300 | ||
|
|
||
| def test_multiple_clients_same_session(self): | ||
| q1 = register_zp_client("c1", "s1") | ||
| q2 = register_zp_client("c2", "s1") | ||
| push_zp_event({"session_id": "s1", "type": "test"}) | ||
| assert q1.qsize() == 1 | ||
| assert q2.qsize() == 1 | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,172 @@ | ||||||
| import { describe, it, expect, vi, beforeEach } from "vitest"; | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add
Suggested change
|
||||||
| import { createSSEClient } from "../sse-client"; | ||||||
|
|
||||||
| function mockReadableStream(chunks: string[]) { | ||||||
| let index = 0; | ||||||
| return { | ||||||
| getReader: () => ({ | ||||||
| read: async () => { | ||||||
| if (index >= chunks.length) return { done: true, value: undefined }; | ||||||
| const value = new TextEncoder().encode(chunks[index++]); | ||||||
| return { done: false, value }; | ||||||
| }, | ||||||
| }), | ||||||
| }; | ||||||
| } | ||||||
|
|
||||||
| describe("sse-client", () => { | ||||||
| beforeEach(() => { | ||||||
| vi.resetModules(); | ||||||
| vi.unstubAllEnvs(); | ||||||
| }); | ||||||
|
|
||||||
| it("parses JSON SSE data lines and calls onEvent", async () => { | ||||||
| const events: Array<{ type: string; data: Record<string, unknown> }> = []; | ||||||
|
|
||||||
| vi.stubGlobal( | ||||||
| "fetch", | ||||||
| vi.fn().mockResolvedValue({ | ||||||
| ok: true, | ||||||
| body: mockReadableStream([ | ||||||
| 'data: {"type":"phase","step":"council"}\n\n', | ||||||
| 'data: {"type":"score","value":85}\n\n', | ||||||
| ]), | ||||||
| }), | ||||||
| ); | ||||||
|
|
||||||
| const abort = createSSEClient({ | ||||||
| url: "http://test/run", | ||||||
| body: { prompt: "test" }, | ||||||
| onEvent: (event) => events.push(event), | ||||||
| onComplete: () => {}, | ||||||
| }); | ||||||
|
|
||||||
| // Wait for async stream processing | ||||||
| await new Promise((r) => setTimeout(r, 50)); | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using a fixed
Suggested change
|
||||||
|
|
||||||
| expect(events.length).toBe(2); | ||||||
| expect(events[0].type).toBe("phase"); | ||||||
| expect(events[1].data).toEqual({ type: "score", value: 85 }); | ||||||
|
|
||||||
| abort(); | ||||||
| }); | ||||||
|
|
||||||
| it("handles plain text SSE data gracefully", async () => { | ||||||
| const events: Array<{ type: string; data: Record<string, unknown> }> = []; | ||||||
|
|
||||||
| vi.stubGlobal( | ||||||
| "fetch", | ||||||
| vi.fn().mockResolvedValue({ | ||||||
| ok: true, | ||||||
| body: mockReadableStream(["data: not-json-content\n\n"]), | ||||||
| }), | ||||||
| ); | ||||||
|
|
||||||
| createSSEClient({ | ||||||
| url: "http://test/run", | ||||||
| body: { prompt: "test" }, | ||||||
| onEvent: (event) => events.push(event), | ||||||
| }); | ||||||
|
|
||||||
| await new Promise((r) => setTimeout(r, 50)); | ||||||
|
|
||||||
| expect(events.length).toBe(1); | ||||||
| expect(events[0].type).toBe("message"); | ||||||
| expect(events[0].data).toEqual({ text: "not-json-content" }); | ||||||
| }); | ||||||
|
|
||||||
| it("calls onError on non-ok response", async () => { | ||||||
| const errors: Error[] = []; | ||||||
|
|
||||||
| vi.stubGlobal( | ||||||
| "fetch", | ||||||
| vi.fn().mockResolvedValue({ | ||||||
| ok: false, | ||||||
| status: 500, | ||||||
| statusText: "Internal Server Error", | ||||||
| }), | ||||||
| ); | ||||||
|
|
||||||
| createSSEClient({ | ||||||
| url: "http://test/run", | ||||||
| body: { prompt: "test" }, | ||||||
| onEvent: () => {}, | ||||||
| onError: (err) => errors.push(err), | ||||||
| }); | ||||||
|
|
||||||
| await new Promise((r) => setTimeout(r, 50)); | ||||||
|
|
||||||
| expect(errors.length).toBe(1); | ||||||
| expect(errors[0].message).toContain("500"); | ||||||
| }); | ||||||
|
|
||||||
| it("calls onComplete when stream ends", async () => { | ||||||
| let completed = false; | ||||||
|
|
||||||
| vi.stubGlobal( | ||||||
| "fetch", | ||||||
| vi.fn().mockResolvedValue({ | ||||||
| ok: true, | ||||||
| body: mockReadableStream([]), | ||||||
| }), | ||||||
| ); | ||||||
|
|
||||||
| createSSEClient({ | ||||||
| url: "http://test/run", | ||||||
| body: { prompt: "test" }, | ||||||
| onEvent: () => {}, | ||||||
| onComplete: () => { | ||||||
| completed = true; | ||||||
| }, | ||||||
| }); | ||||||
|
|
||||||
| await new Promise((r) => setTimeout(r, 50)); | ||||||
|
|
||||||
| expect(completed).toBe(true); | ||||||
| }); | ||||||
|
|
||||||
| it("abort returns a function", () => { | ||||||
| vi.stubGlobal( | ||||||
| "fetch", | ||||||
| vi.fn().mockResolvedValue({ | ||||||
| ok: true, | ||||||
| body: mockReadableStream([]), | ||||||
| }), | ||||||
| ); | ||||||
|
|
||||||
| const abort = createSSEClient({ | ||||||
| url: "http://test/run", | ||||||
| body: { prompt: "test" }, | ||||||
| onEvent: () => {}, | ||||||
| }); | ||||||
|
|
||||||
| expect(typeof abort).toBe("function"); | ||||||
| abort(); // Should not throw | ||||||
| }); | ||||||
|
|
||||||
| it("ignores SSE comment lines starting with colon", async () => { | ||||||
| const events: Array<{ type: string }> = []; | ||||||
|
|
||||||
| vi.stubGlobal( | ||||||
| "fetch", | ||||||
| vi.fn().mockResolvedValue({ | ||||||
| ok: true, | ||||||
| body: mockReadableStream([ | ||||||
| ": this is a comment\n", | ||||||
| 'data: {"type":"real"}\n\n', | ||||||
| ]), | ||||||
| }), | ||||||
| ); | ||||||
|
|
||||||
| createSSEClient({ | ||||||
| url: "http://test/run", | ||||||
| body: { prompt: "test" }, | ||||||
| onEvent: (event) => events.push(event), | ||||||
| }); | ||||||
|
|
||||||
| await new Promise((r) => setTimeout(r, 50)); | ||||||
|
|
||||||
| expect(events.length).toBe(1); | ||||||
| expect(events[0].type).toBe("real"); | ||||||
| }); | ||||||
| }); | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| import { describe, it, expect, vi, beforeEach } from "vitest"; | ||
|
|
||
| describe("zero-prompt-api", () => { | ||
| beforeEach(() => { | ||
| vi.resetModules(); | ||
| vi.unstubAllEnvs(); | ||
| vi.stubGlobal( | ||
| "fetch", | ||
| vi.fn().mockResolvedValue({ | ||
| ok: true, | ||
| json: async () => ({ session_id: "test-session", status: "exploring", cards: [] }), | ||
| text: async () => JSON.stringify({ session_id: "test-session", status: "exploring", cards: [] }), | ||
| }), | ||
| ); | ||
| }); | ||
|
|
||
| it("startSession sends POST with goal", async () => { | ||
| const { startSession } = await import("../zero-prompt-api"); | ||
| const session = await startSession(5); | ||
| expect(session.session_id).toBe("test-session"); | ||
|
|
||
| const [url, init] = (fetch as ReturnType<typeof vi.fn>).mock.calls[0]; | ||
| expect(url).toContain("/zero-prompt/start"); | ||
| expect(init.method).toBe("POST"); | ||
| expect(JSON.parse(init.body)).toEqual({ goal: 5 }); | ||
| }); | ||
|
|
||
| it("startSession parses SSE-wrapped response", async () => { | ||
| vi.stubGlobal( | ||
| "fetch", | ||
| vi.fn().mockResolvedValue({ | ||
| ok: true, | ||
| text: async () => 'data: {"type":"zp.session.start","session_id":"sse-sess","session_status":"exploring","goal_go_cards":3}\n', | ||
| json: async () => ({}), | ||
| }), | ||
| ); | ||
| const { startSession } = await import("../zero-prompt-api"); | ||
| const session = await startSession(3); | ||
| expect(session.session_id).toBe("sse-sess"); | ||
| }); | ||
|
|
||
| it("startSession throws on non-ok response", async () => { | ||
| vi.stubGlobal("fetch", vi.fn().mockResolvedValue({ ok: false, status: 500 })); | ||
| const { startSession } = await import("../zero-prompt-api"); | ||
| await expect(startSession()).rejects.toThrow("Failed to start session"); | ||
| }); | ||
|
|
||
| it("getDashboard returns session data", async () => { | ||
| const { getDashboard } = await import("../zero-prompt-api"); | ||
| const result = await getDashboard(); | ||
| expect(result.session_id).toBe("test-session"); | ||
| }); | ||
|
|
||
| it("queueBuild sends correct action payload", async () => { | ||
| const { queueBuild } = await import("../zero-prompt-api"); | ||
| await queueBuild("session-1", "card-abc"); | ||
|
|
||
| const [url, init] = (fetch as ReturnType<typeof vi.fn>).mock.calls[0]; | ||
| expect(url).toContain("/zero-prompt/session-1/actions"); | ||
| expect(JSON.parse(init.body)).toEqual({ action: "queue_build", card_id: "card-abc" }); | ||
| }); | ||
|
|
||
| it("passCard sends correct action payload", async () => { | ||
| const { passCard } = await import("../zero-prompt-api"); | ||
| await passCard("session-1", "card-xyz"); | ||
|
|
||
| const body = JSON.parse((fetch as ReturnType<typeof vi.fn>).mock.calls[0][1].body); | ||
| expect(body).toEqual({ action: "pass_card", card_id: "card-xyz" }); | ||
| }); | ||
|
|
||
| it("deleteCard sends correct action payload", async () => { | ||
| const { deleteCard } = await import("../zero-prompt-api"); | ||
| await deleteCard("session-1", "card-del"); | ||
|
|
||
| const body = JSON.parse((fetch as ReturnType<typeof vi.fn>).mock.calls[0][1].body); | ||
| expect(body).toEqual({ action: "delete_card", card_id: "card-del" }); | ||
| }); | ||
|
|
||
| it("getBuildEventsUrl includes session and card IDs", async () => { | ||
| const { getBuildEventsUrl } = await import("../zero-prompt-api"); | ||
| const url = getBuildEventsUrl("sess-1", "card-1"); | ||
| expect(url).toContain("/zero-prompt/sess-1/build/card-1/events"); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid hardcoding the queue capacity (300) in the test, as it mirrors a hardcoded value in the implementation. This makes the test fragile to changes in the production code's configuration. It is better to use the
maxsizeproperty of the queue object to ensure the test remains valid even if the capacity is adjusted.