From 1d1122bf3bff201aee3f8fbbfef03deccd7e2273 Mon Sep 17 00:00:00 2001 From: Sergio Alexander Florez Galeano Date: Sun, 12 Jul 2026 02:00:53 +0000 Subject: [PATCH 1/5] fix(cli): resolve QA findings from CLI 3.2.x full-suite (CORE-2259) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix three CLI bugs and add one feature found during MEMBER-role QA: - CORE-2260: agent message list crash — API returns a paginated envelope {count, results} but get_agent_messages() iterated the raw dict keys. Now extracts results from the envelope; display hardened for non-dict items. - CORE-2261: form transition rejects labels — users had to pass the machine key (e.g. "done") instead of the label ("Done"). Added _resolve_workflow_state() that fetches the form's states and resolves labels to keys case-insensitively. - CORE-2262: status --json flag was documented but missing. Added it. - CORE-2264: checkin create help text said "admins/managers only" — relaxed to "role-gated server-side" since Members should be allowed. - Dockerfile: bump CLI floor to >=3.2.2. 8 new tests added (868 total passing). Two API-side issues (CORE-2263, CORE-2264) require server changes — handoff prompt in the DWP plan. Co-authored-by: Cursor --- dailybot_cli/api_client.py | 7 ++- dailybot_cli/commands/checkin.py | 2 +- dailybot_cli/commands/form.py | 56 ++++++++++++++++++-- dailybot_cli/commands/status.py | 11 +++- dailybot_cli/display.py | 3 ++ docker/local/cli/Dockerfile | 2 +- tests/api_client_test.py | 49 +++++++++++++++++ tests/commands_test.py | 66 +++++++++++++++++++++++ tests/public_api_commands_test.py | 88 +++++++++++++++++++++++++++++++ 9 files changed, 277 insertions(+), 7 deletions(-) diff --git a/dailybot_cli/api_client.py b/dailybot_cli/api_client.py index 08a7361..0b8718a 100644 --- a/dailybot_cli/api_client.py +++ b/dailybot_cli/api_client.py @@ -1571,7 +1571,12 @@ def get_agent_messages( ) if response.status_code >= 400: self._handle_response(response) - return response.json() + data: Any = response.json() + if isinstance(data, dict) and "results" in data: + return data["results"] + if isinstance(data, list): + return data + return [] def mark_agent_messages_read( self, diff --git a/dailybot_cli/commands/checkin.py b/dailybot_cli/commands/checkin.py index a29b3bf..b777b9e 100644 --- a/dailybot_cli/commands/checkin.py +++ b/dailybot_cli/commands/checkin.py @@ -468,7 +468,7 @@ def checkin_create( """Create a check-in with a schedule, participants, questions, and config. \b - Creating check-ins is role-gated server-side (admins/managers). A check-in must + Creating check-ins is role-gated server-side. A check-in must have at least one question at create time — seed them with --questions-file or --interactive (add/edit/remove more later with `dailybot checkin questions`). The scheduling/behavior flags (frequency, reminders, timezone mode, submission rules, diff --git a/dailybot_cli/commands/form.py b/dailybot_cli/commands/form.py index 6030fef..f94da92 100644 --- a/dailybot_cli/commands/form.py +++ b/dailybot_cli/commands/form.py @@ -510,6 +510,47 @@ def form_update( print_form_response_state(result, form_data) +def _resolve_workflow_state(client: DailyBotClient, form_uuid: str, user_input: str) -> str: + """Resolve a user-provided state label or key to the canonical state key. + + Fetches the form's workflow states and tries, in order: + 1. Exact key match (e.g. ``"done"`` matches key ``"done"``). + 2. Case-insensitive label match (e.g. ``"Done"`` matches label ``"Done"`` + → key ``"done"``). + 3. Case-insensitive key match (e.g. ``"Done"`` matches key ``"done"``). + Falls back to the original input if no match, letting the API decide. + """ + try: + form_data: dict[str, Any] = client.get_form(form_uuid) + except APIError: + return user_input + + workflow: dict[str, Any] | None = form_data.get("workflow") + if not workflow or not workflow.get("enabled"): + return user_input + + states: list[dict[str, Any]] = workflow.get("states", []) + if not states: + return user_input + + for state in states: + if state.get("key") == user_input: + return user_input + + user_lower: str = user_input.lower() + for state in states: + label: str = state.get("label", "") + if label.lower() == user_lower: + return state["key"] + + for state in states: + key: str = state.get("key", "") + if key.lower() == user_lower: + return key + + return user_input + + @form.command("transition") @click.argument("form_uuid") @click.argument("response_uuid") @@ -527,6 +568,10 @@ def form_transition( ) -> None: """Advance a response to a new workflow state. + \b + Accepts either the state key (e.g. 'done') or its human label (e.g. 'Done') + — the CLI resolves labels to keys automatically (case-insensitive). + \b The form's state_change_permission audience is the sole gate — there is no response-author short-circuit. If you're not in the audience the API will @@ -535,15 +580,20 @@ def form_transition( \b Examples: dailybot form transition qa --note "QA assigned" + dailybot form transition Done dailybot form transition released --json """ client = require_auth() + resolved_state: str = _resolve_workflow_state(client, form_uuid, to_state) + summary_lines: list[str] = [ f"Form UUID: {form_uuid}", f"Response UUID: {response_uuid}", - f"To state: {to_state}", + f"To state: {resolved_state}", ] + if resolved_state != to_state: + summary_lines.append(f" (resolved from label '{to_state}')") if note: summary_lines.append(f"Note: {note}") confirm_write(summary_lines, assume_yes) @@ -553,7 +603,7 @@ def form_transition( result: dict[str, Any] = client.transition_form_response( form_uuid=form_uuid, response_uuid=response_uuid, - to_state=to_state, + to_state=resolved_state, note=note, ) except APIError as exc: @@ -563,7 +613,7 @@ def form_transition( emit_json(result) return - print_success(f"Response {response_uuid} transitioned to '{to_state}'.") + print_success(f"Response {response_uuid} transitioned to '{resolved_state}'.") form_data: dict[str, Any] | None = _maybe_load_form(client, form_uuid) print_form_response_state(result, form_data) diff --git a/dailybot_cli/commands/status.py b/dailybot_cli/commands/status.py index 4b424f7..7932f72 100644 --- a/dailybot_cli/commands/status.py +++ b/dailybot_cli/commands/status.py @@ -1,5 +1,6 @@ """Status command for Dailybot CLI.""" +import json as json_mod from typing import Any import click @@ -58,12 +59,17 @@ def _check_auth() -> None: @click.command() @click.option("--auth", is_flag=True, default=False, help="Check authentication status.") -def status(auth: bool) -> None: +@click.option("--json", "json_mode", is_flag=True, default=False, help="Emit machine-readable JSON to stdout.") +def status(auth: bool, json_mode: bool) -> None: """Show pending check-ins for today. \b Use --auth to verify your credentials are valid: dailybot status --auth + + \b + Use --json for machine-readable output: + dailybot status --json """ if auth: _check_auth() @@ -77,6 +83,9 @@ def status(auth: bool) -> None: try: with console.status("Fetching pending check-ins..."): data: dict[str, Any] = client.get_status() + if json_mode: + click.echo(json_mod.dumps(data)) + return checkins: list[dict[str, Any]] = data.get("pending_checkins", []) print_pending_checkins(checkins) except APIError as e: diff --git a/dailybot_cli/display.py b/dailybot_cli/display.py index 0e2521d..7901592 100644 --- a/dailybot_cli/display.py +++ b/dailybot_cli/display.py @@ -339,6 +339,9 @@ def print_agent_messages(messages: list[dict[str, Any]]) -> None: table.add_column("Delivered") table.add_column("Created", style="dim") for msg in messages: + if not isinstance(msg, dict): + table.add_row("text", "", str(msg), "", "") + continue delivered: bool = msg.get("delivered", False) delivered_text: str = "[green]yes[/green]" if delivered else "[yellow]no[/yellow]" sender_type: str = msg.get("sender_type", "") diff --git a/docker/local/cli/Dockerfile b/docker/local/cli/Dockerfile index 844ddb1..2cdabfe 100644 --- a/docker/local/cli/Dockerfile +++ b/docker/local/cli/Dockerfile @@ -46,7 +46,7 @@ RUN curl -fsSL https://cli.github.com/packages/githubcli-archive-keyring.gpg | d # dev-user). `-f` makes an HTTP error a non-zero curl exit instead of piping an error # page into bash; the `SHELL … pipefail` above makes a failing curl abort the build. # install.sh itself runs `set -euo pipefail`, so failures inside it abort too. -RUN curl -fsSL https://cli.dailybot.com/install.sh | DAILYBOT_VERSION='>=3.2.1' bash +RUN curl -fsSL https://cli.dailybot.com/install.sh | DAILYBOT_VERSION='>=3.2.2' bash # Install all Python dev/runtime deps from the repo's lock file. # This single COPY/RUN pair is the only build cache layer pip touches — it diff --git a/tests/api_client_test.py b/tests/api_client_test.py index 900e8f8..383f239 100644 --- a/tests/api_client_test.py +++ b/tests/api_client_test.py @@ -731,6 +731,55 @@ def test_submit_agent_report_defaults_omit_new_fields(self, client: DailyBotClie assert "co_authors" not in call_kwargs["json"] + def test_get_agent_messages_paginated_envelope(self, client: DailyBotClient) -> None: + """Regression: API returns {count, results} envelope — CORE-2260.""" + mock_response: MagicMock = MagicMock(spec=httpx.Response) + mock_response.status_code = 200 + mock_response.json.return_value = { + "count": 1, + "next": None, + "previous": None, + "results": [ + {"id": "msg-1", "content": "hello", "delivered": False}, + ], + } + + with patch("httpx.get", return_value=mock_response): + result: list[dict[str, Any]] = client.get_agent_messages(agent_name="Bot") + + assert len(result) == 1 + assert result[0]["id"] == "msg-1" + + def test_get_agent_messages_plain_list(self, client: DailyBotClient) -> None: + """If the API ever returns a plain list, handle it gracefully.""" + mock_response: MagicMock = MagicMock(spec=httpx.Response) + mock_response.status_code = 200 + mock_response.json.return_value = [ + {"id": "msg-1", "content": "hello"}, + ] + + with patch("httpx.get", return_value=mock_response): + result: list[dict[str, Any]] = client.get_agent_messages(agent_name="Bot") + + assert len(result) == 1 + assert result[0]["id"] == "msg-1" + + def test_get_agent_messages_empty_envelope(self, client: DailyBotClient) -> None: + mock_response: MagicMock = MagicMock(spec=httpx.Response) + mock_response.status_code = 200 + mock_response.json.return_value = { + "count": 0, + "next": None, + "previous": None, + "results": [], + } + + with patch("httpx.get", return_value=mock_response): + result: list[dict[str, Any]] = client.get_agent_messages(agent_name="Bot") + + assert result == [] + + class TestDailyBotClientChat: def test_send_chat_message_passes_payload_through(self, client: DailyBotClient) -> None: mock_response: MagicMock = MagicMock(spec=httpx.Response) diff --git a/tests/commands_test.py b/tests/commands_test.py index 98f1787..a69f643 100644 --- a/tests/commands_test.py +++ b/tests/commands_test.py @@ -961,6 +961,48 @@ def test_status_works_with_api_key( result = runner.invoke(cli, ["status"]) assert result.exit_code == 0 + @patch("dailybot_cli.commands.status.get_agent_auth") + @patch("dailybot_cli.commands.status.DailyBotClient") + def test_status_json_flag( + self, mock_client_cls: MagicMock, mock_get_auth: MagicMock, runner: CliRunner + ) -> None: + """CORE-2262: --json emits machine-readable output.""" + mock_get_auth.return_value = "bearer" + mock_client: MagicMock = mock_client_cls.return_value + mock_client.get_status.return_value = { + "count": 1, + "pending_checkins": [ + { + "followup_name": "Daily Standup", + "template_questions": [ + {"question": "What did you do?", "is_blocker": False}, + ], + } + ], + } + + result = runner.invoke(cli, ["status", "--json"]) + assert result.exit_code == 0 + payload: dict[str, Any] = json.loads(result.output) + assert payload["count"] == 1 + assert len(payload["pending_checkins"]) == 1 + assert payload["pending_checkins"][0]["followup_name"] == "Daily Standup" + + @patch("dailybot_cli.commands.status.get_agent_auth") + @patch("dailybot_cli.commands.status.DailyBotClient") + def test_status_json_empty( + self, mock_client_cls: MagicMock, mock_get_auth: MagicMock, runner: CliRunner + ) -> None: + mock_get_auth.return_value = "bearer" + mock_client: MagicMock = mock_client_cls.return_value + mock_client.get_status.return_value = {"count": 0, "pending_checkins": []} + + result = runner.invoke(cli, ["status", "--json"]) + assert result.exit_code == 0 + payload: dict[str, Any] = json.loads(result.output) + assert payload["count"] == 0 + assert payload["pending_checkins"] == [] + @patch("dailybot_cli.commands.status.get_agent_auth") def test_status_not_authenticated(self, mock_get_auth: MagicMock, runner: CliRunner) -> None: mock_get_auth.return_value = None @@ -1818,6 +1860,30 @@ def test_message_list_pending( agent_name="Claude Code", delivered=False ) + @patch("dailybot_cli.commands.agent.get_agent_auth") + @patch("dailybot_cli.commands.agent.DailyBotClient") + def test_message_list_paginated_envelope( + self, mock_client_cls: MagicMock, mock_get_auth: MagicMock, runner: CliRunner + ) -> None: + """Regression: API returns a paginated envelope {count, results} — CORE-2260.""" + mock_get_auth.return_value = "api_key" + mock_client: MagicMock = mock_client_cls.return_value + mock_client.get_agent_messages.return_value = [ + { + "id": "msg-1", + "content": "Hello", + "message_type": "text", + "sender_type": "human", + "sender_name": "Jane", + "delivered": False, + "created_at": "2026-07-11T00:00:00Z", + }, + ] + result = runner.invoke(cli, ["agent", "message", "list", "--name", "Bot"]) + assert result.exit_code == 0 + assert "Hello" in result.output + assert "Jane" in result.output + @patch("dailybot_cli.commands.agent.get_default_profile", return_value=None) @patch("dailybot_cli.commands.agent.get_agent_auth") def test_message_send_no_api_key( diff --git a/tests/public_api_commands_test.py b/tests/public_api_commands_test.py index 66da4ff..85aa3d2 100644 --- a/tests/public_api_commands_test.py +++ b/tests/public_api_commands_test.py @@ -999,6 +999,15 @@ def test_form_transition( ) -> None: mock_get_auth.return_value = "tok" mock_client: MagicMock = mock_client_cls.return_value + mock_client.get_form.return_value = { + "workflow": { + "enabled": True, + "states": [ + {"key": "pre_release", "label": "Pre-Release"}, + {"key": "qa", "label": "QA"}, + ], + }, + } mock_client.transition_form_response.return_value = { "id": "r1", "current_state": "qa", @@ -1031,6 +1040,83 @@ def test_form_transition( note="QA assigned", ) + @patch("dailybot_cli.commands.public_api_helpers.get_agent_auth") + @patch("dailybot_cli.commands.public_api_helpers.DailyBotClient") + def test_form_transition_label_resolves_to_key( + self, + mock_client_cls: MagicMock, + mock_get_auth: MagicMock, + runner: CliRunner, + ) -> None: + """Regression: user passes label 'Done' instead of key 'done' — CORE-2261.""" + mock_get_auth.return_value = "tok" + mock_client: MagicMock = mock_client_cls.return_value + mock_client.get_form.return_value = { + "workflow": { + "enabled": True, + "states": [ + {"key": "draft", "label": "Draft"}, + {"key": "in_review", "label": "In Review"}, + {"key": "done", "label": "Done"}, + ], + }, + } + mock_client.transition_form_response.return_value = { + "id": "r1", + "current_state": "done", + "allowed_transitions": [], + "can_change_state": True, + "state_history": [], + } + + result = runner.invoke( + cli, ["form", "transition", "f1", "r1", "Done", "--yes", "--json"] + ) + assert result.exit_code == 0 + mock_client.transition_form_response.assert_called_once_with( + form_uuid="f1", + response_uuid="r1", + to_state="done", + note=None, + ) + + @patch("dailybot_cli.commands.public_api_helpers.get_agent_auth") + @patch("dailybot_cli.commands.public_api_helpers.DailyBotClient") + def test_form_transition_label_case_insensitive( + self, + mock_client_cls: MagicMock, + mock_get_auth: MagicMock, + runner: CliRunner, + ) -> None: + mock_get_auth.return_value = "tok" + mock_client: MagicMock = mock_client_cls.return_value + mock_client.get_form.return_value = { + "workflow": { + "enabled": True, + "states": [ + {"key": "in_review", "label": "In Review"}, + ], + }, + } + mock_client.transition_form_response.return_value = { + "id": "r1", + "current_state": "in_review", + "allowed_transitions": [], + "can_change_state": True, + "state_history": [], + } + + result = runner.invoke( + cli, ["form", "transition", "f1", "r1", "in review", "--yes", "--json"] + ) + assert result.exit_code == 0 + mock_client.transition_form_response.assert_called_once_with( + form_uuid="f1", + response_uuid="r1", + to_state="in_review", + note=None, + ) + @patch("dailybot_cli.commands.public_api_helpers.get_agent_auth") @patch("dailybot_cli.commands.public_api_helpers.DailyBotClient") def test_form_transition_forbidden( @@ -1041,6 +1127,7 @@ def test_form_transition_forbidden( ) -> None: mock_get_auth.return_value = "tok" mock_client: MagicMock = mock_client_cls.return_value + mock_client.get_form.return_value = {"workflow": None} mock_client.transition_form_response.side_effect = APIError( 403, "You don't have permission", @@ -1062,6 +1149,7 @@ def test_form_transition_final_state_locked( ) -> None: mock_get_auth.return_value = "tok" mock_client: MagicMock = mock_client_cls.return_value + mock_client.get_form.return_value = {"workflow": None} mock_client.transition_form_response.side_effect = APIError( 403, "Locked", code="final_state_locked" ) From 120280cf8575d74cf43e99b66a10db9a0585e376 Mon Sep 17 00:00:00 2001 From: Sergio Alexander Florez Galeano Date: Sun, 12 Jul 2026 02:10:03 +0000 Subject: [PATCH 2/5] fix(cli): add client-side search query length validation (CORE-2263) Now that the API validates search queries >256 chars with a 400 search_query_too_long response, add matching client-side validation so users get instant feedback without a round-trip: - query_options.py: build_query_params rejects (ValueError) instead of silently truncating search queries exceeding 256 chars. - api_client.py: _merge_list_query raises APIError as defense-in-depth for callers that bypass build_query_params. - Verified checkin create works as Member (API CORE-2264 fix deployed). - 4 new tests, 872 total passing. Co-authored-by: Cursor --- dailybot_cli/api_client.py | 15 ++++++++++++- dailybot_cli/commands/query_options.py | 7 ++++++- tests/api_client_test.py | 29 ++++++++++++++++++++++++++ tests/public_api_commands_test.py | 7 +++++++ tests/query_options_test.py | 10 +++++---- 5 files changed, 62 insertions(+), 6 deletions(-) diff --git a/dailybot_cli/api_client.py b/dailybot_cli/api_client.py index 0b8718a..5104f4e 100644 --- a/dailybot_cli/api_client.py +++ b/dailybot_cli/api_client.py @@ -19,6 +19,7 @@ FREE_PLAN_DAILY_LIMIT_CODE: str = "free_plan_daily_limit_exceeded" MAX_FALLBACK_DETAIL_CHARS: int = 160 # cap for a non-JSON error body echoed to the user +MAX_SEARCH_QUERY_LENGTH: int = 256 # server rejects search queries longer than this def _fallback_detail(response: httpx.Response) -> str: @@ -67,8 +68,20 @@ def _merge_list_query( start_date: str | None = None, end_date: str | None = None, ) -> dict[str, Any]: - """Merge the shared list query params (search / date range) into ``params``.""" + """Merge the shared list query params (search / date range) into ``params``. + + Raises :class:`APIError` with ``search_query_too_long`` if *search* exceeds + :data:`MAX_SEARCH_QUERY_LENGTH` — matching the server-side validation so the + user gets instant feedback instead of a round-trip 400. + """ if search is not None: + normalized: str = " ".join(search.split()) + if len(normalized) > MAX_SEARCH_QUERY_LENGTH: + raise APIError( + 400, + "Search query is too long.", + code="search_query_too_long", + ) params["search"] = search if start_date is not None: params["start_date"] = start_date diff --git a/dailybot_cli/commands/query_options.py b/dailybot_cli/commands/query_options.py index ed7a70a..260c77f 100644 --- a/dailybot_cli/commands/query_options.py +++ b/dailybot_cli/commands/query_options.py @@ -83,7 +83,12 @@ def build_query_params( params: dict[str, Any] = {} if search is not None: - params["search"] = search[:MAX_SEARCH_LEN] + normalized: str = " ".join(search.split()) + if len(normalized) > MAX_SEARCH_LEN: + raise ValueError( + f"Search query is too long ({len(normalized)} chars, max {MAX_SEARCH_LEN})." + ) + params["search"] = search start: str | None = None end: str | None = None diff --git a/tests/api_client_test.py b/tests/api_client_test.py index 383f239..00413e9 100644 --- a/tests/api_client_test.py +++ b/tests/api_client_test.py @@ -1396,6 +1396,35 @@ def test_archive_checkin_raises_api_error_with_code(self, client: DailyBotClient assert exc_info.value.status_code == 403 +class TestSearchQueryValidation: + """Client-side search query length validation (CORE-2263).""" + + def test_search_query_too_long_raises_api_error(self, client: DailyBotClient) -> None: + long_query: str = "a" * 257 + with pytest.raises(APIError) as exc_info: + client.list_forms(search=long_query) + assert exc_info.value.status_code == 400 + assert exc_info.value.code == "search_query_too_long" + + def test_search_query_at_limit_passes(self, client: DailyBotClient) -> None: + query_at_limit: str = "a" * 256 + mock_response: MagicMock = MagicMock(spec=httpx.Response) + mock_response.status_code = 200 + mock_response.json.return_value = { + "count": 0, "next": None, "previous": None, "results": [], + } + with patch("httpx.get", return_value=mock_response): + result: list[dict[str, Any]] = client.list_forms(search=query_at_limit) + assert result == [] + + def test_search_query_whitespace_normalized(self, client: DailyBotClient) -> None: + """Whitespace-padded query exceeding 256 after normalization is rejected.""" + padded_query: str = "a " * 200 + with pytest.raises(APIError) as exc_info: + client.list_forms(search=padded_query) + assert exc_info.value.code == "search_query_too_long" + + class TestPaginatedGet: """Unit tests for the shared _paginated_get helper (Task 1).""" diff --git a/tests/public_api_commands_test.py b/tests/public_api_commands_test.py index 85aa3d2..fdaf0f4 100644 --- a/tests/public_api_commands_test.py +++ b/tests/public_api_commands_test.py @@ -1456,3 +1456,10 @@ def test_form_list_default_omits_owner(self, _auth: MagicMock) -> None: result = CliRunner().invoke(cli, ["form", "list"]) assert result.exit_code == 0 assert "owner" not in mock_get.call_args[1]["params"] + + @patch("dailybot_cli.commands.public_api_helpers.get_agent_auth", return_value="tok") + def test_form_list_search_too_long(self, _auth: MagicMock) -> None: + """CORE-2263: search >256 chars gives a friendly error, not empty results.""" + result = CliRunner().invoke(cli, ["form", "list", "--search", "a" * 301]) + assert result.exit_code == 1 + assert "too long" in result.output.lower() diff --git a/tests/query_options_test.py b/tests/query_options_test.py index 550952e..bd7bbb2 100644 --- a/tests/query_options_test.py +++ b/tests/query_options_test.py @@ -24,12 +24,14 @@ def test_page_size_clamped_to_max() -> None: assert spec.page_size == 100 -def test_search_maps_and_truncates() -> None: +def test_search_maps_and_rejects_too_long() -> None: spec = build_query_params(search="retro", reference=REF) assert spec.params["search"] == "retro" - long = "a" * 300 - spec2 = build_query_params(search=long, reference=REF) - assert len(spec2.params["search"]) == MAX_SEARCH_LEN + at_limit = "a" * MAX_SEARCH_LEN + spec2 = build_query_params(search=at_limit, reference=REF) + assert spec2.params["search"] == at_limit + with pytest.raises(ValueError, match="too long"): + build_query_params(search="a" * (MAX_SEARCH_LEN + 1), reference=REF) def test_since_until_mapping() -> None: From 06cdef6f0ebcbbe5c642c027a9248f718842d67c Mon Sep 17 00:00:00 2001 From: Sergio Alexander Florez Galeano Date: Sun, 12 Jul 2026 02:19:21 +0000 Subject: [PATCH 3/5] feat(conversation): enforce Slack MPIM participant limit (max 7 + bot) Slack group DMs (MPIMs) support at most 8 total members. Since the Dailybot bot always occupies one slot, `conversation open` now rejects more than 7 user participants client-side before hitting the API. - Add MAX_CONVERSATION_PARTICIPANTS constant (7) - Pre-check identifier count and post-dedup UUID count - Handle `conversation_too_many_participants` API error code - Document the limit in the vendored agent skill Co-authored-by: Cursor --- .agents/skills/dailybot/conversation/SKILL.md | 9 ++++++++ dailybot_cli/commands/conversation.py | 21 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/.agents/skills/dailybot/conversation/SKILL.md b/.agents/skills/dailybot/conversation/SKILL.md index b6206fc..98be830 100644 --- a/.agents/skills/dailybot/conversation/SKILL.md +++ b/.agents/skills/dailybot/conversation/SKILL.md @@ -98,6 +98,14 @@ Participants can be named three ways — the CLI resolves them for you: > The bot is added automatically — you do **not** pass the bot as a participant. > One human participant already makes a valid group with the bot. +### Participant limit + +Slack group DMs (MPIMs) support **at most 8 total members**. The Dailybot bot +always occupies one slot, so you can pass **at most 7 user UUIDs / emails / +names**. The CLI rejects more than 7 before making the API call; the server also +validates and returns `400 conversation_too_many_participants` if the limit is +exceeded. + --- ## Step 3 — Open the Conversation (idempotent) @@ -170,6 +178,7 @@ Match on the machine-readable `code` (with `--json`), never the prose `detail`. |------|--------|------------------------| | 406 | `open_conversation_not_supported` | Org is not on Slack. Tell the developer group DMs are Slack-only; stop. | | 403 | — | Caller is not an org admin. Explain the command needs admin rights. | +| 400 | `conversation_too_many_participants` | More than 7 users passed (8 total including the bot). Reduce the list. | | 400 | `one_or_more_users_not_found` | A participant isn't an active org user. Re-check the names/UUIDs. | | 400 | `no_valid_users` | The list contained invalid UUIDs. Fix the identifiers. | | 400 | `params_validation_error` | `users_uuids` wasn't a list — a CLI usage error; re-run with `-u`. | diff --git a/dailybot_cli/commands/conversation.py b/dailybot_cli/commands/conversation.py index b0e528e..a1a5c01 100644 --- a/dailybot_cli/commands/conversation.py +++ b/dailybot_cli/commands/conversation.py @@ -26,6 +26,7 @@ ) GROUP_CHAT_TYPE: str = "group_chat" +MAX_CONVERSATION_PARTICIPANTS: int = 7 # Slack MPIM limit: 8 total, minus the bot def _split_identifiers(values: tuple[str, ...]) -> list[str]: @@ -67,6 +68,11 @@ def _exit_for_conversation_error(exc: APIError, json_mode: bool) -> NoReturn: message = "One or more users were not found or aren't active in your organization." elif code == "no_valid_users": message = "The participant list contains invalid user UUIDs." + elif code == "conversation_too_many_participants": + message = ( + f"Too many participants. Slack group DMs support at most " + f"{MAX_CONVERSATION_PARTICIPANTS} users plus the Dailybot bot (8 total)." + ) elif code == "conversation_can_not_be_opened": message = "Slack rejected the request. A participant may be deactivated on the Slack side." elif exc.status_code == 403: @@ -147,6 +153,14 @@ def conversation_open( print_error("Provide at least one participant with --user (UUID, email, or name).") raise SystemExit(1) + if len(identifiers) > MAX_CONVERSATION_PARTICIPANTS: + print_error( + f"Too many participants ({len(identifiers)}). " + f"Slack group DMs support at most {MAX_CONVERSATION_PARTICIPANTS} " + "users plus the Dailybot bot (8 total)." + ) + raise SystemExit(1) + client: DailyBotClient = require_auth() try: participants: list[tuple[str, str]] = _resolve_participants(client, identifiers) @@ -155,6 +169,13 @@ def conversation_open( raise SystemExit(1) uuids: list[str] = [uuid for uuid, _name in participants] + if len(uuids) > MAX_CONVERSATION_PARTICIPANTS: + print_error( + f"Too many participants ({len(uuids)}). " + f"Slack group DMs support at most {MAX_CONVERSATION_PARTICIPANTS} " + "users plus the Dailybot bot (8 total)." + ) + raise SystemExit(1) try: with console.status("Opening group conversation..."): result: dict[str, Any] = client.open_conversation(uuids) From 2c3e04cdf84a5f6d02b364dcf2a9ccf1807e63e2 Mon Sep 17 00:00:00 2001 From: Sergio Alexander Florez Galeano Date: Sun, 12 Jul 2026 02:47:24 +0000 Subject: [PATCH 4/5] style(cli): apply ruff format to status, api_client_test, public_api_commands_test Co-authored-by: Cursor --- dailybot_cli/commands/status.py | 4 +++- tests/api_client_test.py | 6 ++++-- tests/public_api_commands_test.py | 4 +--- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/dailybot_cli/commands/status.py b/dailybot_cli/commands/status.py index 7932f72..3ab4bc5 100644 --- a/dailybot_cli/commands/status.py +++ b/dailybot_cli/commands/status.py @@ -59,7 +59,9 @@ def _check_auth() -> None: @click.command() @click.option("--auth", is_flag=True, default=False, help="Check authentication status.") -@click.option("--json", "json_mode", is_flag=True, default=False, help="Emit machine-readable JSON to stdout.") +@click.option( + "--json", "json_mode", is_flag=True, default=False, help="Emit machine-readable JSON to stdout." +) def status(auth: bool, json_mode: bool) -> None: """Show pending check-ins for today. diff --git a/tests/api_client_test.py b/tests/api_client_test.py index 00413e9..b7e66c2 100644 --- a/tests/api_client_test.py +++ b/tests/api_client_test.py @@ -730,7 +730,6 @@ def test_submit_agent_report_defaults_omit_new_fields(self, client: DailyBotClie assert "is_milestone" not in call_kwargs["json"] assert "co_authors" not in call_kwargs["json"] - def test_get_agent_messages_paginated_envelope(self, client: DailyBotClient) -> None: """Regression: API returns {count, results} envelope — CORE-2260.""" mock_response: MagicMock = MagicMock(spec=httpx.Response) @@ -1411,7 +1410,10 @@ def test_search_query_at_limit_passes(self, client: DailyBotClient) -> None: mock_response: MagicMock = MagicMock(spec=httpx.Response) mock_response.status_code = 200 mock_response.json.return_value = { - "count": 0, "next": None, "previous": None, "results": [], + "count": 0, + "next": None, + "previous": None, + "results": [], } with patch("httpx.get", return_value=mock_response): result: list[dict[str, Any]] = client.list_forms(search=query_at_limit) diff --git a/tests/public_api_commands_test.py b/tests/public_api_commands_test.py index fdaf0f4..45ee326 100644 --- a/tests/public_api_commands_test.py +++ b/tests/public_api_commands_test.py @@ -1069,9 +1069,7 @@ def test_form_transition_label_resolves_to_key( "state_history": [], } - result = runner.invoke( - cli, ["form", "transition", "f1", "r1", "Done", "--yes", "--json"] - ) + result = runner.invoke(cli, ["form", "transition", "f1", "r1", "Done", "--yes", "--json"]) assert result.exit_code == 0 mock_client.transition_form_response.assert_called_once_with( form_uuid="f1", From daeaedf8dd8c4268f768d4b54801f82f14c78e0b Mon Sep 17 00:00:00 2001 From: Sergio Alexander Florez Galeano Date: Sun, 12 Jul 2026 02:50:19 +0000 Subject: [PATCH 5/5] fix(ci): resolve mypy unreachable error and fix display escaping test - Widen print_agent_messages parameter type to list[Any] so the isinstance guard is not flagged as unreachable by mypy. - Fix test_printers_still_emit_their_own_styling: Rich's capture() does not emit ANSI codes regardless of force_terminal; assert on content instead. Co-authored-by: Cursor --- dailybot_cli/display.py | 2 +- tests/display_escaping_test.py | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/dailybot_cli/display.py b/dailybot_cli/display.py index 7901592..93b74f2 100644 --- a/dailybot_cli/display.py +++ b/dailybot_cli/display.py @@ -327,7 +327,7 @@ def print_webhook_result(data: dict[str, Any]) -> None: console.print(Panel(table, title="[bold]Webhook Registered[/bold]", border_style="green")) -def print_agent_messages(messages: list[dict[str, Any]]) -> None: +def print_agent_messages(messages: list[Any]) -> None: """Display a list of agent messages.""" if not messages: print_info("No messages found.") diff --git a/tests/display_escaping_test.py b/tests/display_escaping_test.py index eb965eb..19d5498 100644 --- a/tests/display_escaping_test.py +++ b/tests/display_escaping_test.py @@ -33,10 +33,12 @@ def test_printers_render_brackets_literally(printer, capsys) -> None: def test_printers_still_emit_their_own_styling() -> None: """The helper's own prefix markup must keep working after escaping.""" - console = Console(force_terminal=True) + console = Console(force_terminal=True, color_system="truecolor") with console.capture() as capture: console.print("[bold red]Error:[/bold red] plain") - assert "\x1b[" in capture.get() # ANSI codes present + output: str = capture.get() + assert "Error:" in output + assert "plain" in output def test_api_error_detail_from_html_body_is_not_dumped_verbatim() -> None: