fix(lists): shared-project records appear, and a list's q means what search means
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 8s
CI & Build / TypeScript typecheck (push) Successful in 12s
CI & Build / integration (push) Successful in 18s
CI & Build / Python tests (push) Successful in 48s
CI & Build / Build & push image (push) Successful in 28s
CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 8s
CI & Build / TypeScript typecheck (push) Successful in 12s
CI & Build / integration (push) Successful in 18s
CI & Build / Python tests (push) Successful in 48s
CI & Build / Build & push image (push) Successful in 28s
#2462, both decided halves. ## The ACL defect list_notes filtered on Note.user_id == user_id with no scope parameter at all — never a per-call decision, the capability was absent. query_knowledge beside it was deliberately browse-scoped with a comment saying why. So a task in a shared project was invisible in list_tasks, enter_project's open-task list, the SessionStart todo count and the web UI's task views, while the same project's notes and snippets appeared. Now the shared clause: notes_visibility_clause(user_id, "browse"). Browse and not read, per decision #2094 — an ambient list must never surface a record someone shared one-to-one; those stay search-only. This is the remaining half of a fix made twice before (#2159 widened fetch, #2092 widened meaning), and the guard below is what stops a fourth half appearing. Guarded by source inspection in test_retrieval_scopes: every list-shaped service references a shared visibility clause AND carries no bare Note.user_id comparison that would quietly re-narrow it. An owner-only list returns correct-looking rows and simply omits the shared ones — the shape no behavioural test catches. ## q is semantic (operator: "make it match") The UI's note list keyword-matched while the UI's Browse search semantic-matched, over the same records, with nothing saying so. Now one meaning: q joins the embedding index and orders by cosine distance at the interactive floor, with every lifecycle filter still applied in the same indexed query. Relevance ordering wins over `sort` when q is present — a query is a relevance claim, and sorting its results by date would shuffle the answer. ILIKE survives only as the embedder-down fallback: degraded, never empty. Stated position: superseded records are NOT demoted in list-q. The penalty reorders a top-k; reordering a paginated, counted list would make page boundaries lie. The search surfaces carry the demotion. ## The interactive floor becomes one constant INTERACTIVE_SEARCH_THRESHOLD = 0.3 in embeddings.py, consumed by routes/search.py (was a commented constant), knowledge.py (was a bare literal), and the new list-q. This closes #2463's finding 3 early — the same number lived in two files with the reasoning attached to only one. ## Deferred, deliberately The return-shape unification (ORM objects vs dicts) stays undone. It is a 14-caller refactor whose motivation — callers being unable to swap paths — shrinks now that both paths share the clause and the meaning of q. If swap pressure recurs it deserves its own change, not a rider on an ACL fix. Refs #2462, #2463
This commit is contained in:
@@ -202,3 +202,46 @@ async def test_injected_menu_labels_the_record_kind():
|
||||
assert "body" not in out["context"]
|
||||
# The header can't claim they're all notes when the markers say otherwise.
|
||||
assert "Scribe records" in lines[0]
|
||||
|
||||
|
||||
def test_list_shaped_services_use_the_shared_visibility_clause():
|
||||
"""Every service that LISTS notes must declare its reach through
|
||||
notes_visibility_clause / browsable_notes_clause — never a bare owner
|
||||
filter (#47, #2462).
|
||||
|
||||
Source inspection, because this is the shape no behavioural test catches:
|
||||
an owner-only list returns correct-looking rows and simply omits the shared
|
||||
ones, which is how list_notes shipped owner-only for months while
|
||||
query_knowledge beside it was deliberately browse-scoped. A task in a
|
||||
shared project was invisible in list_tasks, enter_project and the web UI
|
||||
while the same project's notes appeared.
|
||||
"""
|
||||
import ast
|
||||
import inspect
|
||||
import textwrap
|
||||
|
||||
from scribe.services import knowledge, notes
|
||||
|
||||
for fn, label in ((notes.list_notes, "notes.list_notes"),
|
||||
(knowledge.query_knowledge, "knowledge.query_knowledge")):
|
||||
source = textwrap.dedent(inspect.getsource(fn))
|
||||
assert (
|
||||
"notes_visibility_clause" in source
|
||||
or "browsable_notes_clause" in source
|
||||
), (
|
||||
f"{label} does not reference a shared visibility clause — a bare "
|
||||
f"owner filter silently hides shared-project records (#2462)"
|
||||
)
|
||||
# And it must not ALSO carry a bare owner filter on the main query,
|
||||
# which would quietly re-narrow whatever the clause granted.
|
||||
tree = ast.parse(source)
|
||||
for node in ast.walk(tree):
|
||||
if (isinstance(node, ast.Compare)
|
||||
and isinstance(node.left, ast.Attribute)
|
||||
and node.left.attr == "user_id"
|
||||
and isinstance(node.left.value, ast.Name)
|
||||
and node.left.value.id == "Note"):
|
||||
raise AssertionError(
|
||||
f"{label} compares Note.user_id directly — reach must come "
|
||||
f"from the shared clause, not a bare owner filter"
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user