From 49753d61d791c8620a83091150a8cdb12be5ca9c Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 16 Aug 2026 16:21:39 +0000 Subject: [PATCH 1/2] fix: keep requester thread-group runs behind the cutoff (v0.86.2) A thread-group run the signed-in account requested no longer lists when that thread has no ABAC-visible post at knowledge_cutoff. Same- named R&R people bind the earliest catalog row instead of unordered LIMIT 1. Co-authored-by: Seongho Bae --- CHANGELOG.d/0.86.2-requester-thread-cutoff.md | 3 + CHANGELOG.md | 12 ++ CLAUDE.md | 4 + backend/app/analysis_run_ingestion.py | 55 +++++---- backend/app/main.py | 6 +- backend/app/post_summary_ingestion.py | 5 +- backend/tests/test_api.py | 80 +++++++++++++ docs/adr/0009-cross-post-actor-identity.md | 3 + docs/adr/0018-related-nodes-team-org-walk.md | 6 + frontend/package.json | 2 +- lineageweave/__init__.py | 2 +- pyproject.toml | 2 +- tests/test_analysis_run_authorization.py | 112 +++++++++++++++--- tests/test_ingestion_transaction_contracts.py | 19 +++ tests/test_person_mention_projection.py | 71 ++++++++++- uv.lock | 2 +- 16 files changed, 336 insertions(+), 48 deletions(-) create mode 100644 CHANGELOG.d/0.86.2-requester-thread-cutoff.md diff --git a/CHANGELOG.d/0.86.2-requester-thread-cutoff.md b/CHANGELOG.d/0.86.2-requester-thread-cutoff.md new file mode 100644 index 000000000..50426f9cf --- /dev/null +++ b/CHANGELOG.d/0.86.2-requester-thread-cutoff.md @@ -0,0 +1,3 @@ +A thread-group run you requested stays off the home list unless that +thread has an in-cutoff visible post. Same-named R&R people bind the +earliest catalog row. diff --git a/CHANGELOG.md b/CHANGELOG.md index fd8717513..f59a1c42c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,18 @@ All notable changes to this project are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); versioning follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.86.2] - 2026-08-16 + +### Fixed + +- A thread-group analysis-run you requested no longer appears on the + home list when that thread has no ABAC-visible post at or before + `knowledge_cutoff` (ADR 0018). Open a thread that has an in-cutoff + visible post, then request again. Detail of the hidden row is 404. +- An R&R person name that matches two catalog rows now binds the + earliest `created_at`, then `person_id`. A later same-named Keyman + row no longer steals the mention. + ## [0.86.1] - 2026-08-16 ### Changed diff --git a/CLAUDE.md b/CLAUDE.md index a11127584..9b2371118 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -20,3 +20,7 @@ Opening a cutoff title shows the live post -- compare it with the cutoff before treating the body as reconstructed evidence (ADR 0016). `POST /api/analysis-runs` records Pending on an authorized cutoff capture (ADR 0017) and does not reconstruct lineage. +A thread-group run lists only when an ABAC-visible post exists at or +before `knowledge_cutoff`, even when the signed-in account requested +the run (ADR 0018). Requesting a January thread that has no in-cutoff +visible post does not put that row on the home list. diff --git a/backend/app/analysis_run_ingestion.py b/backend/app/analysis_run_ingestion.py index d26eb6f6e..25cf7730e 100644 --- a/backend/app/analysis_run_ingestion.py +++ b/backend/app/analysis_run_ingestion.py @@ -1,10 +1,11 @@ """Authorized, source-redacting reads of the Milestone 2 analysis-run registry. The registry itself is issue #89 / migration 0018. This module is the -product projection: an account sees only runs they requested or whose -scope they already have ABAC authority to walk. Aggregate counts and -lookup labels come back; source SQL, DSNs, raw records, and provider -payloads never do. +product projection: an account sees runs they requested or whose scope +they already have ABAC authority to walk, except a thread-group run +also needs an in-cutoff visible post (ADR 0018). Requester ownership +does not bypass that clock. Aggregate counts and lookup labels come +back; source SQL, DSNs, raw records, and provider payloads never do. ``create_pending_analysis_run`` (ADR 0017) writes snapshot, counts, run, scope, and the first Pending event atomically. It does not reconstruct @@ -33,23 +34,8 @@ "analysis_run_tepp": "tepp-run-v1", } -_VISIBLE_RUN_SQL = """ - run.requested_by_account_id = $1 - or ( - scope.scope_kind_code = 'analysis_scope_corporate_entity' - and scope.corporate_entity_id = any($2::uuid[]) - ) - or ( - scope.scope_kind_code = 'analysis_scope_process_unit' - and exists ( - select 1 from account_affiliation aff - where aff.user_account_id = $1 - and aff.process_unit_id = scope.process_unit_id - ) - ) - or ( - scope.scope_kind_code = 'analysis_scope_thread_group' - and exists ( +_THREAD_GROUP_IN_CUTOFF_SQL = """ + exists ( select 1 from source_post p where p.thread_group_key = scope.scope_key and p.created_at <= run.knowledge_cutoff @@ -58,6 +44,31 @@ or p.corporate_entity_id = any($2::uuid[]) ) ) +""" + +_VISIBLE_RUN_SQL = f""" + ( + run.requested_by_account_id = $1 + or ( + scope.scope_kind_code = 'analysis_scope_corporate_entity' + and scope.corporate_entity_id = any($2::uuid[]) + ) + or ( + scope.scope_kind_code = 'analysis_scope_process_unit' + and exists ( + select 1 from account_affiliation aff + where aff.user_account_id = $1 + and aff.process_unit_id = scope.process_unit_id + ) + ) + or ( + scope.scope_kind_code = 'analysis_scope_thread_group' + and {_THREAD_GROUP_IN_CUTOFF_SQL} + ) + ) + and ( + scope.scope_kind_code <> 'analysis_scope_thread_group' + or {_THREAD_GROUP_IN_CUTOFF_SQL} ) """ @@ -203,7 +214,7 @@ async def fetch_visible_analysis_runs( account_id: str, affiliated_entity_ids: list[str], ) -> list[dict[str, Any]]: - """Runs the account requested or whose scope they may already walk.""" + """Runs the account may walk, with thread-group cutoff still applied.""" rows = await conn.fetch( _RUN_SELECT.format(where=_VISIBLE_RUN_SQL), account_id, diff --git a/backend/app/main.py b/backend/app/main.py index adb7a20a8..013fd6899 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -1194,8 +1194,10 @@ async def list_analysis_runs( ) -> dict[str, Any]: """Authorized analysis-run list: aggregates and labels only. - Hidden scopes 404 at the item path and never appear here. The - payload has no source SQL, DSN, raw record, or provider body. + Hidden scopes 404 at the item path and never appear here. A + thread-group run you requested still needs an in-cutoff visible + post (ADR 0018). The payload has no source SQL, DSN, raw record, + or provider body. """ _require_post_read(account) async with pool.acquire() as conn: diff --git a/backend/app/post_summary_ingestion.py b/backend/app/post_summary_ingestion.py index 36d4fadae..7043f1e62 100644 --- a/backend/app/post_summary_ingestion.py +++ b/backend/app/post_summary_ingestion.py @@ -258,7 +258,10 @@ async def _replace_summary_projection( ) elif role.actor_type_code == ACTOR_TYPE_PERSON: person_row = await conn.fetchrow( - "select person_id from cataloged_person where person_name = $1 limit 1", + "select person_id from cataloged_person " + "where person_name = $1 " + "order by created_at, person_id " + "limit 1", role.actor_name, ) if person_row is not None: diff --git a/backend/tests/test_api.py b/backend/tests/test_api.py index bff7f7c64..d6e5114d9 100644 --- a/backend/tests/test_api.py +++ b/backend/tests/test_api.py @@ -1607,6 +1607,86 @@ def test_thread_group_run_list_honors_knowledge_cutoff( assert seeded_db["visible_run_id"] in ids +def test_requester_owned_thread_group_run_list_honors_knowledge_cutoff( + client, demo_analyst_token, seeded_db +) -> None: + """Requesting a January thread-group run does not list it without an in-cutoff post.""" + + admin_conn = psycopg2.connect(seeded_db["dsn"]) + admin_conn.autocommit = True + try: + with admin_conn.cursor() as cur: + cur.execute( + "insert into source_post (author_account_id, corporate_entity_id, post_title, post_body, voc_type_code, visibility_code, thread_group_key, created_at) " + "select author_account_id, corporate_entity_id, %s, %s, 'voc', 'public', %s, %s " + "from source_post where post_id = %s", + ( + "Late requester thread-group post", + "Written after the January cutoff.", + "late-requester-thread-group", + "2026-01-20T12:00:00Z", + seeded_db["own_private_post_id"], + ), + ) + cur.execute( + """ + insert into analysis_source_snapshot + (snapshot_sha256, source_contract_version, + maximum_available_time, captured_at) + values (%s, 'source-contract-v1', + '2026-01-12T00:00:00Z', '2026-01-12T00:05:00Z') + returning analysis_source_snapshot_id + """, + ("e" * 64,), + ) + snapshot_id = cur.fetchone()[0] + cur.execute( + """ + insert into analysis_run + (analysis_source_snapshot_id, run_kind_code, idempotency_key, + requested_by_account_id, knowledge_cutoff, + configuration_schema_version, configuration_sha256, + code_revision_sha, requested_at) + values (%s, 'analysis_run_lineage', %s, + (select user_account_id from user_account + where email_address = 'test.analyst@example.test'), + '2026-01-12T12:00:00Z', 'lineage-run-v1', %s, %s, + '2026-01-12T12:30:00Z') + returning analysis_run_id + """, + (snapshot_id, "hidden-own-late-thread", "b" * 64, "c" * 40), + ) + run_id = str(cur.fetchone()[0]) + cur.execute( + """ + insert into analysis_run_scope + (analysis_run_id, scope_kind_code, scope_key) + values (%s, 'analysis_scope_thread_group', 'late-requester-thread-group') + """, + (run_id,), + ) + cur.execute( + """ + insert into analysis_run_status_event + (analysis_run_id, status_ordinal, status_code, occurred_at) + values (%s, 1, 'analysis_status_succeeded', '2026-01-12T12:33:00Z') + """, + (run_id,), + ) + finally: + admin_conn.close() + + headers = {"Authorization": f"Bearer {demo_analyst_token}"} + listed = client.get("/api/analysis-runs", headers=headers) + assert listed.status_code == 200 + ids = {run["analysis_run_id"] for run in listed.json()["analysis_runs"]} + assert run_id not in ids + assert seeded_db["visible_run_id"] in ids + + hidden = client.get(f"/api/analysis-runs/{run_id}", headers=headers) + assert hidden.status_code == 404 + + def test_first_mention_of_a_new_counterparty_creates_a_real_corporate_entity( client, demo_analyst_token, seeded_db, monkeypatch ) -> None: diff --git a/docs/adr/0009-cross-post-actor-identity.md b/docs/adr/0009-cross-post-actor-identity.md index 7bdbfa091..d53bda606 100644 --- a/docs/adr/0009-cross-post-actor-identity.md +++ b/docs/adr/0009-cross-post-actor-identity.md @@ -44,6 +44,9 @@ counterparty and Keyman affiliation already resolves against). **Person** (an R&R actor, not a Keyman): opportunistically joined to an *existing* `cataloged_person` row by exact name match, when Keyman extraction has already cataloged that name on this or another post. +When two rows share `person_name`, the join takes the earliest +`created_at`, then `person_id` — it does not invent a person or pick +an arbitrary `LIMIT 1` row. R&R does not create a new person identity itself -- `cataloged_person` requires `person_side_code` (our-side vs. counterparty), which R&R's prompt does not currently ask for and Keyman's does; inventing one here diff --git a/docs/adr/0018-related-nodes-team-org-walk.md b/docs/adr/0018-related-nodes-team-org-walk.md index 17e4236a6..76ae064ee 100644 --- a/docs/adr/0018-related-nodes-team-org-walk.md +++ b/docs/adr/0018-related-nodes-team-org-walk.md @@ -39,6 +39,9 @@ post. The popup turns that name into a related-node button. Thread-group run list visibility requires at least one ABAC-visible `source_post` whose `created_at` is at or before `knowledge_cutoff`. +Requester ownership (`requested_by_account_id`) does not bypass that +clock: a run you requested on a thread that has no in-cutoff visible +post stays off the home list and returns 404 on detail. ## Consequences @@ -48,6 +51,9 @@ Thread-group run list visibility requires at least one ABAC-visible organization chip. - A later public post in a thread group no longer lists a January run that could not have known that post. +- Requesting that January run yourself does not put it on your home + list. Open a thread that has an in-cutoff visible post, then request + again. ## References diff --git a/frontend/package.json b/frontend/package.json index 803acd91b..fb52f7948 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,7 +1,7 @@ { "name": "frontend", "private": true, - "version": "0.86.1", + "version": "0.86.2", "type": "module", "scripts": { "dev": "vite", diff --git a/lineageweave/__init__.py b/lineageweave/__init__.py index 48bf6e481..efe84890b 100644 --- a/lineageweave/__init__.py +++ b/lineageweave/__init__.py @@ -55,4 +55,4 @@ "sentence_excerpts", ] -__version__ = "0.86.1" +__version__ = "0.86.2" diff --git a/pyproject.toml b/pyproject.toml index eb2e26318..6e41c7ca7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "lineageweave" -version = "0.86.1" +version = "0.86.2" description = "Reconstructs git-branch-style lineage DAGs from scattered short records using multi-channel score fusion and LLM adjudication." readme = "README.md" license = { text = "MIT" } diff --git a/tests/test_analysis_run_authorization.py b/tests/test_analysis_run_authorization.py index 730825c14..be71e6357 100644 --- a/tests/test_analysis_run_authorization.py +++ b/tests/test_analysis_run_authorization.py @@ -11,6 +11,8 @@ import pytest from psycopg2 import sql +from backend.app.analysis_run_ingestion import _VISIBLE_RUN_SQL + _ROOT = Path(__file__).resolve().parents[1] _INITIAL_MIGRATION = _ROOT / "migrations" / "0001_initial_schema.sql" _REGISTRY_MIGRATION = _ROOT / "migrations" / "0018_analysis_run_registry.sql" @@ -107,6 +109,8 @@ def _complete_run( idempotency_key: str, scope_kind: str, corporate_entity_id: str | None = None, + scope_key: str | None = None, + knowledge_cutoff: str = "2026-01-12T12:00:00Z", ) -> str: """Insert one succeeded run with one document-count aggregate.""" cursor.execute( @@ -137,11 +141,18 @@ def _complete_run( configuration_schema_version, configuration_sha256, code_revision_sha, requested_at) values (%s, 'analysis_run_lineage', %s, %s, - '2026-01-12T12:00:00Z', 'lineage-run-v1', %s, %s, + %s, 'lineage-run-v1', %s, %s, '2026-01-12T12:30:00Z') returning analysis_run_id """, - (snapshot_id, idempotency_key, account_id, "b" * 64, "c" * 40), + ( + snapshot_id, + idempotency_key, + account_id, + knowledge_cutoff, + "b" * 64, + "c" * 40, + ), ) run_id = str(cursor.fetchone()[0]) if scope_kind == "analysis_scope_corporate_entity": @@ -153,6 +164,15 @@ def _complete_run( """, (run_id, scope_kind, corporate_entity_id), ) + elif scope_kind == "analysis_scope_thread_group": + cursor.execute( + """ + insert into analysis_run_scope + (analysis_run_id, scope_kind_code, scope_key) + values (%s, %s, %s) + """, + (run_id, scope_kind, scope_key), + ) else: cursor.execute( """ @@ -180,31 +200,30 @@ def _complete_run( def _visible_ids(cursor, account_id: str, entity_ids: list[str]) -> set[str]: """Apply the same visibility predicate the product API uses.""" + predicate = _VISIBLE_RUN_SQL.replace("$2::uuid[]", "%s::uuid[]").replace( + "$1", "%s" + ) cursor.execute( - """ + f""" select run.analysis_run_id from analysis_run run join analysis_run_scope scope on scope.analysis_run_id = run.analysis_run_id - where - run.requested_by_account_id = %s - or ( - scope.scope_kind_code = 'analysis_scope_corporate_entity' - and scope.corporate_entity_id = any(%s::uuid[]) - ) - or ( - scope.scope_kind_code = 'analysis_scope_process_unit' - and exists ( - select 1 from account_affiliation aff - where aff.user_account_id = %s - and aff.process_unit_id = scope.process_unit_id - ) - ) + where {predicate} """, - (account_id, entity_ids, account_id), + (account_id, entity_ids), ) return {str(row[0]) for row in cursor.fetchall()} +def test_requester_ownership_cannot_bypass_thread_group_cutoff() -> None: + """ADR 0018: requester ownership is not a standalone OR past the clock.""" + + sql = " ".join(_VISIBLE_RUN_SQL.split()) + assert "p.created_at <= run.knowledge_cutoff" in sql + assert "scope.scope_kind_code <> 'analysis_scope_thread_group'" in sql + assert "run.requested_by_account_id = $1" in sql + + def test_hidden_scope_does_not_leak_through_all_visible_or_other_corp(authz_db) -> None: """A Demo-Corp viewer never sees another tenant's run or its aggregates.""" with authz_db.cursor() as cursor: @@ -252,3 +271,60 @@ def test_hidden_scope_does_not_leak_through_all_visible_or_other_corp(authz_db) assert hidden_all_visible in outsider_visible assert hidden_other_corp in outsider_visible assert own_run not in outsider_visible + + +def test_requester_owned_thread_group_run_needs_in_cutoff_post(authz_db) -> None: + """A January thread-group run you requested stays hidden without an in-cutoff post.""" + + with authz_db.cursor() as cursor: + viewer = _insert_account(cursor, "requester") + own_corp = _insert_corp(cursor, "DEMO-CORP-CUTOFF", "Demo Corp") + cursor.execute( + """ + insert into common_lookup_value + (lookup_category, lookup_code, lookup_label) + values + ('post_visibility', 'public', 'Public'), + ('voc_type', 'voc', 'Voice of Customer') + on conflict (lookup_code) do nothing + """ + ) + cursor.execute( + """ + insert into account_affiliation (user_account_id, corporate_entity_id) + values (%s, %s) + """, + (viewer, own_corp), + ) + cursor.execute( + """ + insert into source_post + (author_account_id, corporate_entity_id, post_title, post_body, + voc_type_code, visibility_code, thread_group_key, created_at) + values + (%s, %s, 'Late own thread', 'Written after the January cutoff.', + 'voc', 'public', 'late-own-thread', '2026-01-20T12:00:00Z'), + (%s, %s, 'In-cutoff own thread', 'Visible at the January cutoff.', + 'voc', 'public', 'in-cutoff-own-thread', '2026-01-10T12:00:00Z') + """, + (viewer, own_corp, viewer, own_corp), + ) + hidden_own = _complete_run( + cursor, + account_id=viewer, + digest="1" * 64, + idempotency_key="own-late-thread", + scope_kind="analysis_scope_thread_group", + scope_key="late-own-thread", + ) + visible_own = _complete_run( + cursor, + account_id=viewer, + digest="2" * 64, + idempotency_key="own-in-cutoff-thread", + scope_kind="analysis_scope_thread_group", + scope_key="in-cutoff-own-thread", + ) + visible = _visible_ids(cursor, viewer, [own_corp]) + assert hidden_own not in visible + assert visible_own in visible diff --git a/tests/test_ingestion_transaction_contracts.py b/tests/test_ingestion_transaction_contracts.py index 641a43cc6..3ae5b8d32 100644 --- a/tests/test_ingestion_transaction_contracts.py +++ b/tests/test_ingestion_transaction_contracts.py @@ -199,6 +199,7 @@ async def fetchrow(self, query: str, *args: Any) -> dict[str, Any] | None: return {"korean_summary": "합성 요약"} if compact.startswith("select person_id from cataloged_person"): assert self.in_transaction + assert "order by created_at, person_id" in compact return None raise AssertionError(f"unexpected fetchrow query: {compact}") @@ -215,6 +216,8 @@ async def fetch(self, query: str, *args: Any) -> list[dict[str, Any]]: "responsibility": "도면 검토", "actor_type_code": ACTOR_TYPE_TEAM, "affiliated_organization_name": "Synthetic Energy", + "team_id": None, + "corporate_entity_id": None, } ] raise AssertionError(f"unexpected fetch query: {compact}") @@ -469,3 +472,19 @@ def test_release_notes_describe_balanced_outer_emphasis_stripping() -> None: assert "strips balanced outer Markdown emphasis from field values" in content assert "while still accepting emphasized field labels" in content assert "preserves Markdown emphasis in field values" not in content + + +def test_person_role_join_orders_catalog_homonyms() -> None: + """ADR 0009: two same-named people must not attach by unordered LIMIT 1.""" + + source = ( + Path(__file__).resolve().parents[1] + / "backend" + / "app" + / "post_summary_ingestion.py" + ).read_text(encoding="utf-8") + assert "order by created_at, person_id" in source + assert ( + "select person_id from cataloged_person where person_name = $1 limit 1" + not in source + ) diff --git a/tests/test_person_mention_projection.py b/tests/test_person_mention_projection.py index 5353c2a91..8fdff3fc1 100644 --- a/tests/test_person_mention_projection.py +++ b/tests/test_person_mention_projection.py @@ -36,7 +36,11 @@ NODE_POST, NODE_TEAM, ) -from lineageweave.post_summary import PostSummary, RoleResponsibility +from lineageweave.post_summary import ( + ACTOR_TYPE_PERSON, + PostSummary, + RoleResponsibility, +) _ADMIN_DSN = os.environ.get( "LINEAGEWEAVE_TEST_POSTGRES_ADMIN_DSN", "postgresql://localhost/postgres" @@ -451,3 +455,68 @@ def test_team_only_posts_walk_related_nodes(projection_database: str) -> None: database_dsn, post_id, _summary_person_id = projection_database.split("|") asyncio.run(_exercise_team_only_related_walk(database_dsn, post_id)) + + +async def _exercise_person_homonym_role_binding( + database_dsn: str, + post_id: str, +) -> None: + """A later same-named catalog person must not steal the R&R mention.""" + + connection = await asyncpg.connect(database_dsn) + try: + earlier_id = str( + await connection.fetchval( + """ + insert into cataloged_person + (person_name, person_side_code, created_at) + values ('Homonym Person', 'our_side', '2026-01-01T00:00:00Z') + returning person_id + """ + ) + ) + later_id = str( + await connection.fetchval( + """ + insert into cataloged_person + (person_name, person_side_code, created_at) + values ('Homonym Person', 'counterparty', '2026-02-01T00:00:00Z') + returning person_id + """ + ) + ) + await persist_post_summary( + connection, + post_id, + PostSummary( + korean_summary="동명이인 담당자가 일정을 확인했다.", + roles_and_responsibilities=( + RoleResponsibility( + actor_name="Homonym Person", + responsibility="일정 확인", + actor_type_code=ACTOR_TYPE_PERSON, + ), + ), + ), + ) + mention_ids = [ + str(row["person_id"]) + for row in await connection.fetch( + "select person_id from post_summary_person_mention " + "where post_id = $1", + post_id, + ) + ] + assert mention_ids == [earlier_id] + assert later_id not in mention_ids + finally: + await connection.close() + + +def test_person_role_binds_the_earliest_same_named_catalog_id( + projection_database: str, +) -> None: + """ADR 0009: two catalog people can share a name; R&R keeps the earliest id.""" + + database_dsn, post_id, _summary_person_id = projection_database.split("|") + asyncio.run(_exercise_person_homonym_role_binding(database_dsn, post_id)) diff --git a/uv.lock b/uv.lock index c759df267..e1d2860cf 100644 --- a/uv.lock +++ b/uv.lock @@ -454,7 +454,7 @@ wheels = [ [[package]] name = "lineageweave" -version = "0.86.1" +version = "0.86.2" source = { virtual = "." } dependencies = [ { name = "certifi" }, From 8bab79ee8f241204a0560d66c7ac640977fa7efa Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 16 Aug 2026 16:37:56 +0000 Subject: [PATCH 2/2] fix: bind analysis-run authz tests with named SQL params asyncpg reuses $1/$2 in the thread-group cutoff predicate. Rewriting those tokens to positional %s left five placeholders and two binds, so the ADR 0018 helper could not execute. Named psycopg2 placeholders keep the product SQL as the source of truth. Copy no longer tells the buyer to request a thread-group run that create still refuses. Co-authored-by: Seongho Bae --- CHANGELOG.d/0.86.3-requester-thread-cutoff.md | 5 +- CHANGELOG.md | 6 +- backend/app/main.py | 4 +- docs/adr/0018-related-nodes-team-org-walk.md | 5 +- tests/test_analysis_run_authorization.py | 195 ++++++++++++++++-- tests/test_ingestion_transaction_contracts.py | 24 +++ 6 files changed, 214 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.d/0.86.3-requester-thread-cutoff.md b/CHANGELOG.d/0.86.3-requester-thread-cutoff.md index 50426f9cf..e75a5e3b6 100644 --- a/CHANGELOG.d/0.86.3-requester-thread-cutoff.md +++ b/CHANGELOG.d/0.86.3-requester-thread-cutoff.md @@ -1,3 +1,4 @@ A thread-group run you requested stays off the home list unless that -thread has an in-cutoff visible post. Same-named R&R people bind the -earliest catalog row. +thread already has an in-cutoff visible post. Request still records a +corporate-scope lineage run. Same-named R&R people bind the earliest +catalog row. diff --git a/CHANGELOG.md b/CHANGELOG.md index 471028da4..09ea5b05d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,8 +10,10 @@ All notable changes to this project are documented here. Format follows - A thread-group analysis-run you requested no longer appears on the home list when that thread has no ABAC-visible post at or before - `knowledge_cutoff` (ADR 0018). Open a thread that has an in-cutoff - visible post, then request again. Detail of the hidden row is 404. + `knowledge_cutoff` (ADR 0018). The row appears when that thread + already has an in-cutoff visible post. Request still records a + corporate-scope lineage run (ADR 0017). Detail of the hidden row + is 404. - An R&R person name that matches two catalog rows now binds the earliest `created_at`, then `person_id`. A later same-named Keyman row no longer steals the mention. diff --git a/backend/app/main.py b/backend/app/main.py index 013fd6899..fb95ccacd 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -1263,7 +1263,9 @@ async def read_analysis_run( ) -> dict[str, Any]: """One authorized analysis-run projection, or 404 when hidden. - Detail adds the labeled status history. Hidden runs never leak events. + Detail adds the labeled status history. Hidden runs never leak + events. A thread-group run you requested still needs an in-cutoff + visible post (ADR 0018). """ _require_post_read(account) try: diff --git a/docs/adr/0018-related-nodes-team-org-walk.md b/docs/adr/0018-related-nodes-team-org-walk.md index 6a2193f44..5fcd0d55b 100644 --- a/docs/adr/0018-related-nodes-team-org-walk.md +++ b/docs/adr/0018-related-nodes-team-org-walk.md @@ -53,8 +53,9 @@ post stays off the home list and returns 404 on detail. - A later public post in a thread group no longer lists a January run that could not have known that post. - Requesting that January run yourself does not put it on your home - list. Open a thread that has an in-cutoff visible post, then request - again. + list. The row appears when the thread already has an in-cutoff + visible post. Request still records a corporate-scope lineage run + (ADR 0017). ## References diff --git a/tests/test_analysis_run_authorization.py b/tests/test_analysis_run_authorization.py index be71e6357..a89ae7d96 100644 --- a/tests/test_analysis_run_authorization.py +++ b/tests/test_analysis_run_authorization.py @@ -101,6 +101,37 @@ def _insert_corp(cursor, code: str, name: str) -> str: return str(cursor.fetchone()[0]) +def _insert_process_unit( + cursor, corporate_entity_id: str, code: str, name: str +) -> str: + """Insert one synthetic process unit under a corporate entity.""" + cursor.execute( + """ + insert into process_unit + (corporate_entity_id, process_unit_code, process_unit_name) + values (%s, %s, %s) + returning process_unit_id + """, + (corporate_entity_id, code, name), + ) + return str(cursor.fetchone()[0]) + + +def _ensure_post_lookups(cursor) -> None: + """Insert the visibility and VOC codes the cutoff fixtures need.""" + cursor.execute( + """ + insert into common_lookup_value + (lookup_category, lookup_code, lookup_label) + values + ('post_visibility', 'public', 'Public'), + ('post_visibility', 'private', 'Private'), + ('voc_type', 'voc', 'Voice of Customer') + on conflict (lookup_code) do nothing + """ + ) + + def _complete_run( cursor, *, @@ -109,6 +140,7 @@ def _complete_run( idempotency_key: str, scope_kind: str, corporate_entity_id: str | None = None, + process_unit_id: str | None = None, scope_key: str | None = None, knowledge_cutoff: str = "2026-01-12T12:00:00Z", ) -> str: @@ -173,6 +205,15 @@ def _complete_run( """, (run_id, scope_kind, scope_key), ) + elif scope_kind == "analysis_scope_process_unit": + cursor.execute( + """ + insert into analysis_run_scope + (analysis_run_id, scope_kind_code, process_unit_id) + values (%s, %s, %s) + """, + (run_id, scope_kind, process_unit_id), + ) else: cursor.execute( """ @@ -198,30 +239,62 @@ def _complete_run( return run_id +def _product_visibility_predicate() -> str: + """Translate asyncpg ``$1``/``$2`` reuse into psycopg2 named placeholders. + + The product SQL binds the account once as ``$1`` and affiliated + entity ids once as ``$2``, then reuses both. Positional ``%s`` + cannot do that: interpolating the cutoff fragment twice yields + five placeholders and only two arguments. + """ + return ( + _VISIBLE_RUN_SQL.replace("$2::uuid[]", "%(entity_ids)s::uuid[]").replace( + "$1", "%(account_id)s" + ) + ) + + def _visible_ids(cursor, account_id: str, entity_ids: list[str]) -> set[str]: """Apply the same visibility predicate the product API uses.""" - predicate = _VISIBLE_RUN_SQL.replace("$2::uuid[]", "%s::uuid[]").replace( - "$1", "%s" - ) cursor.execute( f""" select run.analysis_run_id from analysis_run run join analysis_run_scope scope on scope.analysis_run_id = run.analysis_run_id - where {predicate} + where {_product_visibility_predicate()} """, - (account_id, entity_ids), + {"account_id": account_id, "entity_ids": entity_ids}, ) return {str(row[0]) for row in cursor.fetchall()} +def test_visible_ids_helper_reuses_named_account_and_entity_binds() -> None: + """asyncpg $1/$2 reuse must not become positional %s.""" + + predicate = _product_visibility_predicate() + leftover = ( + predicate.replace("%(account_id)s", "").replace("%(entity_ids)s", "") + ) + assert "%(account_id)s" in predicate + assert "%(entity_ids)s::uuid[]" in predicate + assert "%s" not in leftover + assert predicate.count("%(account_id)s") == 2 + assert predicate.count("%(entity_ids)s") == 3 + + def test_requester_ownership_cannot_bypass_thread_group_cutoff() -> None: - """ADR 0018: requester ownership is not a standalone OR past the clock.""" + """ADR 0018: requester ownership stays inside the first conjunct.""" sql = " ".join(_VISIBLE_RUN_SQL.split()) + ownership = "run.requested_by_account_id = $1" + first, separator, second = sql.partition(") and (") + assert ownership in sql assert "p.created_at <= run.knowledge_cutoff" in sql - assert "scope.scope_kind_code <> 'analysis_scope_thread_group'" in sql - assert "run.requested_by_account_id = $1" in sql + assert separator + assert ownership in first + assert ownership not in second + assert "scope.scope_kind_code <> 'analysis_scope_thread_group'" in second + assert "p.created_at <= run.knowledge_cutoff" in second def test_hidden_scope_does_not_leak_through_all_visible_or_other_corp(authz_db) -> None: @@ -279,16 +352,7 @@ def test_requester_owned_thread_group_run_needs_in_cutoff_post(authz_db) -> None with authz_db.cursor() as cursor: viewer = _insert_account(cursor, "requester") own_corp = _insert_corp(cursor, "DEMO-CORP-CUTOFF", "Demo Corp") - cursor.execute( - """ - insert into common_lookup_value - (lookup_category, lookup_code, lookup_label) - values - ('post_visibility', 'public', 'Public'), - ('voc_type', 'voc', 'Voice of Customer') - on conflict (lookup_code) do nothing - """ - ) + _ensure_post_lookups(cursor) cursor.execute( """ insert into account_affiliation (user_account_id, corporate_entity_id) @@ -328,3 +392,98 @@ def test_requester_owned_thread_group_run_needs_in_cutoff_post(authz_db) -> None visible = _visible_ids(cursor, viewer, [own_corp]) assert hidden_own not in visible assert visible_own in visible + + +def test_thread_group_cutoff_honors_private_and_empty_affiliation(authz_db) -> None: + """Private own-corp in-cutoff lists; other-corp private and empty $2 do not.""" + + with authz_db.cursor() as cursor: + viewer = _insert_account(cursor, "private-viewer") + other = _insert_account(cursor, "other-author") + own_corp = _insert_corp(cursor, "DEMO-CORP-PRIVATE", "Demo Corp") + other_corp = _insert_corp(cursor, "OTHER-CORP-PRIVATE", "Other Corp") + _ensure_post_lookups(cursor) + cursor.execute( + """ + insert into account_affiliation (user_account_id, corporate_entity_id) + values (%s, %s) + """, + (viewer, own_corp), + ) + cursor.execute( + """ + insert into source_post + (author_account_id, corporate_entity_id, post_title, post_body, + voc_type_code, visibility_code, thread_group_key, created_at) + values + (%s, %s, 'Private own thread', 'Visible only to Demo Corp.', + 'voc', 'private', 'private-own-thread', '2026-01-10T12:00:00Z'), + (%s, %s, 'Private other thread', 'Other corp only.', + 'voc', 'private', 'private-other-thread', '2026-01-10T12:00:00Z'), + (%s, %s, 'Public empty-aff thread', 'Public in-cutoff post.', + 'voc', 'public', 'public-empty-aff-thread', '2026-01-10T12:00:00Z') + """, + (viewer, own_corp, other, other_corp, viewer, own_corp), + ) + private_own = _complete_run( + cursor, + account_id=viewer, + digest="3" * 64, + idempotency_key="private-own-thread", + scope_kind="analysis_scope_thread_group", + scope_key="private-own-thread", + ) + private_other = _complete_run( + cursor, + account_id=viewer, + digest="4" * 64, + idempotency_key="private-other-thread", + scope_kind="analysis_scope_thread_group", + scope_key="private-other-thread", + ) + public_empty = _complete_run( + cursor, + account_id=viewer, + digest="5" * 64, + idempotency_key="public-empty-aff-thread", + scope_kind="analysis_scope_thread_group", + scope_key="public-empty-aff-thread", + ) + affiliated = _visible_ids(cursor, viewer, [own_corp]) + assert private_own in affiliated + assert private_other not in affiliated + assert public_empty in affiliated + + unaffiliated = _visible_ids(cursor, viewer, []) + assert private_own not in unaffiliated + assert private_other not in unaffiliated + assert public_empty in unaffiliated + + +def test_process_unit_run_lists_without_thread_group_cutoff(authz_db) -> None: + """A process-unit run the caller already walks is not gated by cutoff.""" + + with authz_db.cursor() as cursor: + viewer = _insert_account(cursor, "unit-viewer") + own_corp = _insert_corp(cursor, "DEMO-CORP-UNIT", "Demo Corp") + process_unit_id = _insert_process_unit( + cursor, own_corp, "DEMO-PU-CUTOFF", "Demo Process Unit" + ) + cursor.execute( + """ + insert into account_affiliation + (user_account_id, corporate_entity_id, process_unit_id) + values (%s, %s, %s) + """, + (viewer, own_corp, process_unit_id), + ) + unit_run = _complete_run( + cursor, + account_id=viewer, + digest="6" * 64, + idempotency_key="own-process-unit", + scope_kind="analysis_scope_process_unit", + process_unit_id=process_unit_id, + ) + visible = _visible_ids(cursor, viewer, [own_corp]) + assert unit_run in visible diff --git a/tests/test_ingestion_transaction_contracts.py b/tests/test_ingestion_transaction_contracts.py index 4f208497e..76273b10a 100644 --- a/tests/test_ingestion_transaction_contracts.py +++ b/tests/test_ingestion_transaction_contracts.py @@ -484,6 +484,30 @@ def test_release_notes_describe_balanced_outer_emphasis_stripping() -> None: assert "preserves Markdown emphasis in field values" not in content +def test_release_notes_do_not_offer_thread_group_request() -> None: + """ADR 0017 create is still corporate-scope; copy must not invent a retry path.""" + + changelog = " ".join( + (Path(__file__).resolve().parents[1] / "CHANGELOG.md") + .read_text(encoding="utf-8") + .split() + ) + adr = " ".join( + ( + Path(__file__).resolve().parents[1] + / "docs" + / "adr" + / "0018-related-nodes-team-org-walk.md" + ) + .read_text(encoding="utf-8") + .split() + ) + assert "then request again" not in changelog + assert "then request again" not in adr + assert "Request still records a corporate-scope lineage run" in changelog + assert "Request still records a corporate-scope lineage run" in adr + + def test_person_role_join_orders_catalog_homonyms() -> None: """ADR 0009: two same-named people must not attach by unordered LIMIT 1."""