Repository navigation
fix(chat): work-item binding keeps a stale company scope after membership revocation #13729
Description
Found in review of #13728. Recorded rather than fixed there — it is a bounded deviation, and the fix is a policy decision about where membership is re-checked.
Problem
SessionWorkItemService.set_work_item (chat_workflow/session_work_item.py) stores the company_id the bind endpoint authorised, and the goal-ancestry lookup later trusts it for SESSION_WORK_ITEM_TTL_SECONDS (default 24h). Nothing re-checks membership at resolve time, and nothing invalidates bindings when membership changes.
So a user removed from a company keeps receiving that company's goal chain in their prompt for up to the remaining TTL.
What is already safe
- Work item moved to another company — fails closed.
WorkItemService.get(company_id=<stale>)returnsNone, so the chain is empty. - Cross-company ancestor edges — closed in feat(chat): server-side work-item binding and tenant-scoped goal lookups (#13704, #13687) #13728: the walk stops at a foreign ancestor and
GoalService.createnow rejects the edge.
The gap is specifically revocation of the caller's own membership.
Assessment
Bounded (TTL), requires prior legitimate authorisation, and exposes one goal chain — which is why it did not block #13728. It is still a genuine deviation from "the tenant scope is trustworthy", which is the claim that PR rests on.
Options
- Re-check membership at resolve time — most correct, costs a membership lookup per turn on bound sessions only.
get_tenant_context(api/user_management/dependencies.py:155-224) already does a realllc_company_membershipscheck and could be reused. - Invalidate bindings on membership change — cheapest at read time, but needs a hook on every revocation path and misses any path that forgets.
- Shorten the TTL — reduces the window without closing it. Mitigation, not a fix.
Option 1 is the honest one; option 3 is a reasonable interim if the per-turn cost is unacceptable.
Acceptance criteria
- A user whose membership is revoked stops receiving that company's goal ancestry on their next turn, not at TTL expiry.
- A test proves it: bind, revoke, resolve → empty chain.
- If option 3 is chosen instead, the residual window is stated in
session_work_item.pybeside the TTL constant, so the next reader sees the trade rather than inferring it.
Related
- feat(chat): server-side work-item binding and tenant-scoped goal lookups (#13704, #13687) #13728 / fix(memory): L4 GoalAncestry needs a server-side session→work-item binding and a tenant-scoped goal lookup #13704 — added the binding and its stored scope
- fix(memory): L4 GoalAncestry can never render — ancestry chain never passed at the only call site #13687 — the L4 render this scope protects
Activity
fix(chat): re-check membership when resolving a bound work item (#13729…
mrveiss commentedon Aug 9, 2026
Delivered and merged: PR #13782, squashed as
026a210.Option 1 (re-check at resolve time), with one thing the issue's framing did not cover:
resolve_goal_ancestry(session_id)has no user. The binding stored(work_item_id, company_id)and the call site passes only a session id, so there was nothing to check membership of. Rather than thread a user throughllm_handler→TieredContextBuilder→ the resolver, the binding now records who was authorised next to what — the same reasoning that putcompany_idthere in #13704, one field further. No signature above it changes.MembershipService.is_memberalready existed, so no new mechanism. (The issue pointed at_check_org_membershipinapi/user_management/dependencies.py; importing an API-layer private fromchat_workflowwould be the wrong direction.)Two cases the issue did not name, both of which would have made a naive fix wrong:
get_tenant_contextallows it), so a membership-only check would silently kill L4 for them. The admin flag is checked too — read from the database, never from the stored binding, so losing admin also takes effect on the next turn rather than reintroducing the same staleness for a smaller group.user_idand are refused, not grandfathered — they expire within the TTL anyway, and grandfathering means continuing to honour exactly the unverifiable claim this fixes.test_a_revoked_member_gets_no_chain_on_the_next_turn: bind, revoke, resolve →None, with_query_goal_ancestryasserted not awaited, so the walk does not run and get discarded.Mutation check (deleting the two-line re-check): 2 failed, 13 passed.
The malformed-id guard moved ahead of the membership lookup so a client-shaped mistake does not buy a DB round-trip. Worth noting: an existing prompt test used
"wi-1"as a work-item id, which no longer survives that guard — switching it to a real UUID confirms the guard is live on the real prompt path, not just in unit isolation.Closing.