From 4a93c8b26244a1838ee99a5f906a30b8fc6f4720 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 24 Sep 2026 06:48:01 -0400 Subject: [PATCH] fix(task-logs): a collaborator who may write a task may log on it (rule 78) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit create_log filtered on Note.user_id == user_id, a bare owner check, so a collaborator with write access to a shared task was told it did not exist — and, since a log now stamps the claim, could never be seen working it. It now asks can_write_note. Editing and deleting a log still require its author, which is authorship rather than access. Co-Authored-By: Claude Opus 5.5 --- src/scribe/services/task_logs.py | 12 +++++++----- tests/test_task_claims.py | 16 ++++++++++++++++ tests/test_task_document_shape.py | 4 +++- 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/src/scribe/services/task_logs.py b/src/scribe/services/task_logs.py index e2335fb..fc96d3e 100644 --- a/src/scribe/services/task_logs.py +++ b/src/scribe/services/task_logs.py @@ -7,7 +7,7 @@ from sqlalchemy import func, select from scribe.models import async_session from scribe.models.task_log import TaskLog from scribe.models.note import Note, TaskStatus -from scribe.services.access import can_read_note, readable_notes_clause +from scribe.services.access import can_read_note, can_write_note, readable_notes_clause from scribe.services.task_claims import stamp_claim logger = logging.getLogger(__name__) @@ -51,11 +51,13 @@ async def create_log( content: str, duration_minutes: int | None = None, ) -> TaskLog: + # Whoever may WRITE the task may log on it (rule #78) — a collaborator on a + # shared project included. This used to be a bare owner filter, which + # refused exactly the person a shared task exists for. + if not await can_write_note(user_id, task_id): + raise ValueError(f"Task {task_id} not found") async with async_session() as session: - # Verify task exists and belongs to user - result = await session.execute( - select(Note).where(Note.id == task_id, Note.user_id == user_id) - ) + result = await session.execute(select(Note).where(Note.id == task_id)) task = result.scalars().first() if task is None: raise ValueError(f"Task {task_id} not found") diff --git a/tests/test_task_claims.py b/tests/test_task_claims.py index bbb7426..ffe0580 100644 --- a/tests/test_task_claims.py +++ b/tests/test_task_claims.py @@ -283,3 +283,19 @@ async def test_ending_a_session_releases_only_that_sessions_claims(users): assert (await notes_svc.get_note(owner, gone.id)).claimed_at is None assert (await notes_svc.get_note(owner, kept.id)).claim_session == "sess-stays" assert await tc.release_session(owner, "") == 0 + + +@pytest.mark.integration +async def test_a_collaborator_with_write_access_can_log_and_so_claims(users): + """create_log used to filter on the task's OWNER (a rule #78 violation), so + a collaborator on a shared task could not log on it — nor, since logging + stamps the claim, ever be seen working it.""" + from scribe.services import sharing, task_logs + + owner, collaborator = users + task = await notes_svc.create_note(owner, title="shared work", status="todo") + await sharing.share_note(owner, task.id, target_user_id=collaborator, + permission="editor") + await task_logs.create_log(collaborator, task.id, "picked this up") + got = await notes_svc.get_note(owner, task.id) + assert got.claimed_by == collaborator diff --git a/tests/test_task_document_shape.py b/tests/test_task_document_shape.py index 92dc9f0..affa0ee 100644 --- a/tests/test_task_document_shape.py +++ b/tests/test_task_document_shape.py @@ -125,7 +125,7 @@ def test_a_short_task_with_a_short_log_is_still_one_sharp_chunk(): # is #4241's half-surface one layer down: the entry is readable, and the search # still answers as though it were never written. -from unittest.mock import MagicMock, patch # noqa: E402 +from unittest.mock import AsyncMock, MagicMock, patch # noqa: E402 import pytest # noqa: E402 @@ -141,6 +141,7 @@ async def test_writing_a_work_log_refreshes_the_tasks_embedding(): session = session_returning(note) with ( patch.object(svc, "async_session", return_value=session), + patch.object(svc, "can_write_note", AsyncMock(return_value=True)), patch("scribe.services.notes.embed_note") as embed, ): await svc.create_log(42, 7, "what I tried") @@ -193,6 +194,7 @@ async def test_a_failed_refresh_does_not_fail_the_log_that_saved(): session = session_returning(fake_note(id=7, title="T", body="b", is_task=True)) with ( patch.object(svc, "async_session", return_value=session), + patch.object(svc, "can_write_note", AsyncMock(return_value=True)), patch("scribe.services.notes.embed_note", side_effect=RuntimeError("boom")), ): log = await svc.create_log(42, 7, "what I tried")