fix(operator-queue): a broad list under an agent key filters before the limit and says its total

[AndriiPasternak31]

Fixes abilityai/trinity-enterprise#815

Why

An agent checks whether it, or a permitted peer, already asked a person about something by calling list_operator_queue with no agent_name. Under an agent-scoped key the backend ranked the key owner's whole fleet (pending first, then priority, then newest), cut it at limit, and only then did the MCP drop rows from agents outside {self} ∪ permitted. The caller's own rows and its peers' rows could sit below the cut, so count < limit read as "that's everything" when it was not. A second filter had the same flaw: a machine key's "about a person" rows (gate-, workspace-problem-, portal-inbox-collision-) were dropped after the limit too, which could empty a machine's page entirely (system key included). An agent that concludes "never asked" asks again, and the person gets two approvals for one decision.

What changes

The rule is #3059's: every visibility filter runs in SQL, in the shared WHERE builder (_list_conditions), before LIMIT, so the page, total and the flag counts describe the same rows.

Backend — GET /api/operator-queue

• Agent-key narrowing. An mcp_scope == "agent" key is narrowed to {self} ∪ agent_permissions (∩ the owner's accessible set) in SQL — pure DB, keyed on mcp_scope, never acting_agent_name(), so a system key is not narrowed. An agent scope with no agent name is 403. This is narrowing for completeness, not an authorization point: the MCP's checkAgentAccess stays the gate; /stats, /{id} and /agents/{name} are not narrowed; enforcement on the raw routes stays with trinity-enterprise#629.
• agent_names (new, repeatable, ≤ 500, no blanks → 422): narrowing only — intersected with whatever the caller may see. Applies to the page, total and the flags.
• About-a-person exclusion in SQL for every non-person principal, on both list routes (/agents/{name} too, for parity). It equals the Python rule (is_about_a_person): case-insensitive, leading whitespace stripped exactly as str.strip() does (all 29 str.isspace() characters), U+212A and U+0130 rewritten to Python's lower() form first, a NULL request_id kept. That holds on SQLite and on a UTF-8, non-Turkic PostgreSQL; under a tr/az collation a legacy upper-case row can reach the Python projection, which stays as a belt — a belt drop nulls total and adds a warning.
• Response fields for every principal: total (a COUNT over the same WHERE), has_more (read off a limit + 1 page, never the count), next_offset, next_cursor, plus warnings only when something is degraded. Additive — items, count and the flag counts are unchanged.
• Opt-in cursor walk. cursor=start, then each page's next_cursor. Within one walk no item is returned twice, and every item that matches when the walk starts and when the walk reaches it is returned exactly once, under concurrent inserts, endings and permission changes. Rows are ordered "as of" a watermark W = walk start − 300 s, so a row answered mid-walk keeps its pending-section key; pending platform alerts are ordered by a per-walk priority snapshot in Redis (TTL 1 h; an alert raised mid-walk ranks last), so a re-prioritised alert cannot repeat or be skipped. The token is opaque, unsigned and carries no authority (every page re-applies the caller's visibility). Refusals: malformed token → 422 naming cursor; cursor with a non-zero offset → 422; a token reused with different filters → 422; expired walk → 410 "restart with cursor=start"; Redis unreachable or failing → 503. A sort key too long to carry ends the page with has_more: true, next_cursor: null and a warning. Without cursor the route is in offset mode, ordered exactly as before — the Operations UI and every existing caller see no change.
• The walk's bound: a pending→ended write must commit within 300 s of the timestamp it stamped. SQLite's busy timeout keeps that in practice; PostgreSQL has no statement timeout configured, so each of the five ending writers logs at error when it commits past the margin. An unfiltered walk lists a row answered in the 5 minutes before it began in the pending section, with its real status.

Backend — GET /api/agents/{name}/permissions?strict=true (new, opt-in): built from ONE agent_container_states() snapshot; 503 when Docker cannot be read, instead of the lenient path's "200, no peers". Without the flag the endpoint is unchanged (frontend and the other MCP callers).

MCP — list_operator_queue

• A broad read under an agent key reads its permits first through the strict endpoint and passes them as agent_names, so the page covers exactly what the tool delivers. A failed permissions read is {error: "permissions_unavailable", retryable: true} — never a silent self-only answer; over 500 names or an 8 KB request target is permit_set_too_large, never an unfiltered read.
• The output always carries count, total, has_more, next_cursor, next_offset, items. A field an older backend does not send becomes null with "completeness not verified" (version skew). The post-filter stays as a belt; if it ever drops a row, total becomes null with a warning.
• New opt-in cursor parameter; a walk re-reads the permits on every page. The description states the contract (1,432 characters; the fix(mcp): fit deploy_local_agent and get_objectives inside Claude Code's 2,048-char description cap (#3234) #3238 budget census stays green).

Behaviour changes to note

• Raw REST, agent-scoped key: the broad list is now {self} ∪ permitted, not the owner's whole queue; an explicit agent_name for a non-permitted agent returns items: [], total: 0 (the same answer whether the agent exists or not). Narrower, never wider.
• Deploy backend and MCP together. An older backend makes the new MCP report null paging fields with a warning; an older MCP ignores the new fields.

Tests

• tests/unit/test_ent815_broad_list_agent_scope.py (28) — the real router over a real SQLite file and real agent_permissions rows: the window test (stranger rows saturating limit=10 cannot hide the caller's or a permitted peer's rows; admin and non-admin owner), persons / user / system keys not narrowed, stranger agent_name → empty page, flags narrowed, agent_names only narrows, the SQL exclusion incl. every isspace() lead and the two code-point rewrites (also under a PostgreSQL-style lower), has_more from the page, a forced belt drop, strict permissions, dialect compile.
• tests/unit/test_ent815_queue_walk.py (30) — inserts ahead of / behind the cursor, all five pending→ended writers between pages, a status=pending walk, a permission change, alert priority changes and an alert raised mid-walk, a hypothesis property test over random interleavings, the commit-lag bound and its error log, the strict codec (422/410/503), a forged token never widening, over-cap keys, offset order unchanged, shared expressions on both dialects.
• src/mcp-server/src/operator_queue_list.test.ts (20).
• Mutation-checked through the real route: removing the agent-key narrowing turns 8 tests red (incl. the window test); removing the SQL exclusion turns 9 red; each review-round fix was shown red first.
• Local (targeted, Python 3.14 venv): the two new files plus the 88 existing test files of the touched modules → 2903 passed, 3 skipped (seeds 1 and 2); MCP npm test 740/740 and npm run build clean; lint_sys_modules / lint_root_test_placement clean. Full suites run in CI.

Not verified

• PostgreSQL is covered by compiling every new statement for both dialects; CI runs no PostgreSQL pytest, so live PG behaviour (multi-byte ltrim, collation of tie order) is unproven.
• Redis is exercised through fakeredis only.
• No real-data check of legacy key lengths (max(length(id)), max(length(created_at))) was run. A key too long for a cursor is loud by design (has_more: true, next_cursor: null, a warning), so this only decides how often that warning can appear.

Sequencing

#3255 has merged and is in this branch's base, so no rebase is expected for it. This does not stack on #3256; the MCP tests are in a new file so the two cannot conflict textually.

Out of scope (seen, not done here)

• list_reports filters after the limit → fix(mcp): a broad list_reports under an agent key filters after the backend's limit #3300
• list_recent_executions filters status after the limit → fix(mcp): list_recent_executions applies its status filter after the limit #3301
• The Operations / fleet-grid / mobile queue windows truncate silently → fix(ui): the Operations queue windows truncate silently past their row limit #3302
• Permit enforcement on /stats, /{id} and /agents/{name} (trinity-enterprise#629)
• The case-sensitive _not_prefixed used by the sweeps (unchanged on purpose)
• Enum validation for status / type / priority (a typo silently returns an empty list)
• The other MCP broad lists that post-filter after limit
• Durable decision identity (feat(operator-queue): an agent sees its pending asks and can replace one (#3247) #3256's territory) and a belt-drop metric
• Cursor paging on /agents/{name} (no offset there today, no machine caller)

[AndriiPasternak31]

@vybe, one design call in this PR that I want you to see explicitly. GET /api/operator-queue now narrows an agent-scoped key's broad list to {self} ∪ permitted in the backend (pure DB, the same shape as ent#727's metrics cross-read), so a raw REST agent call no longer gets its owner's whole queue. architecture/security.md §5 said current_user.agent_name is read only by notifications and event subscriptions; this PR updates that line.

It is narrowing for completeness, not authorization: the MCP's checkAgentAccess stays the gate, and /stats, /{id} and /agents/{name} are not narrowed (ent#629 owns enforcement there). The MCP also passes its own permit set as a narrowing-only agent_names, so the page is complete over exactly what it delivers.

Please confirm or object. If you object, the backend half comes out cleanly: the MCP half stands on its own.

[dolho]

/review Report

Branch: AndriiPasternak31/ent815 → dev (merge-base a72271d9c, reviewed at head e9a085a41)
Files Changed: 25 (+2916/-103), of which about 700 lines are source and the rest are tests and docs
Scope: CLEAN
Plan Completion: 6 done / 0 partial / 0 not done / 0 changed / 0 unverifiable (against the linked issue's acceptance criteria)

Intent: a broad list_operator_queue under an agent-scoped key must filter to {self} ∪ permitted before LIMIT, and must report whether the page is complete.
Delivered: agent-key narrowing and the about-a-person exclusion both move into the shared SQL WHERE builder (_list_conditions). The route gains total, has_more, next_offset and next_cursor, an opt-in keyset walk, and a strict permissions read. The MCP tool sends its permit set as agent_names and keeps the old post-filter as a safety belt. The frontend-store sibling named in the issue is deferred to #3302 and listed as out of scope, which is a reasonable split.

Acceptance criterion	Status	Evidence
Agent-scope predicate applied before LIMIT	DONE	services/operator_queue_service.py::narrow_to_agent_key → accessible_agent_names → _list_conditions
Uses the existing seam	DONE	accessible_agent_names plus a new exclude_request_id_prefixes condition, both in _list_conditions
Response tells "everything" apart from "truncated"	DONE	list_for_principal: has_more read from a limit + 1 page; total is a COUNT over the same WHERE
Offset paging returns each row exactly once	DONE	test_b3a_* and test_b3b_*
Regression test for a peer's rows below a saturated window	DONE	test_b1_peer_and_own_rows_below_a_saturated_window_are_returned
Tool description states the completeness guarantee	DONE	tools/operator_queue.ts description

Execution coverage (Step 2.5)

changed symbol / test file	executed by	live consumer	verdict
narrow_to_agent_key / _effective_agents	test_b1, test_b3b, test_b6, test_b9, test_b10, test_b12 (real router over a real SQLite file)	list_for_principal ← routers/operator_queue.py::list_queue_items	✅ executed
_request_id_not_prefixed_ci / exclude_request_id_prefixes	test_b5, test_b7, test_b8, test_b16, test_b17, plus dialect compile in test_b11	_list_conditions (both list routes)	✅ executed
list_for_principal (paging fields, belt drop)	test_b2, test_b13, test_b14	GET /api/operator-queue	✅ executed
list_items_walk / _walk_sort_keys / walk_alert_priorities / cursor codec	test_ent815_queue_walk.py (30, including a Hypothesis interleaving property)	list_for_principal cursor branch	✅ executed
_note_commit_lag (5 writers)	commit-lag tests in test_ent815_queue_walk.py	respond / cancel / batch cancel ×2 / expire writers	✅ executed
_strict_permissions / ?strict=true	test_b15_strict_permissions_never_answer_a_docker_fault_with_no_peers	client.ts::getPermittedAgents(…, {strict: true}) ← list_operator_queue	✅ executed
MCP list_operator_queue / pagedOutput / operatorQueueListTarget	src/mcp-server/src/operator_queue_list.test.ts (20)	MCP tool registry	✅ executed

The source-text grep over the changed test files found no hits in the new tests. The only hits are in pre-existing tests/registry.json description strings.

Fix mutation: I ran these locally, reverting each fix in a scratch working tree and then restoring it.

• I made narrow_to_agent_key return accessible unchanged. 6 tests went red: test_b1 (both admin and non-admin owner), test_b3b, test_b6, test_b9 and test_b10.
• I removed the _request_id_not_prefixed_ci condition from _list_conditions. 6 tests went red (from test_b5, test_b7, test_b8 and test_b16).
• I stopped the MCP tool from sending agent_names. 5 of 20 MCP tests went red.

Local runs: both new pytest files gave 58 passed. The MCP file gave 20/20 passed, and tsc --noEmit is clean. CI checks on the PR are green.

Critical Findings (block merge)

None.

Informational Findings (review required)

[I1] Dead code & consistency: an existing JSDoc block now documents the wrong symbol (Confidence: 9/10)
File: src/mcp-server/src/client.ts:112-122
The new OperatorQueueListParams interface and operatorQueueListTarget were inserted between the existing /** Default trigger set for the #914 /chat recovery lookup … */ comment and export const CHAT_RECOVERY_TRIGGERS. Two doc blocks now sit back to back. Editors and TypeDoc will attach the #914/#2661 rationale to the interface, and CHAT_RECOVERY_TRIGGERS is left undocumented. That rationale explains why the trigger set must not be widened, so losing it is a real cost.
Suggestion: move the two new declarations above that comment block, or below TASK_RECOVERY_TRIGGERS.

[I2] Performance: every list call on the polled route now runs one extra query (Confidence: 6/10)
File: src/backend/services/operator_queue_service.py (list_for_principal): total = db.count_operator_queue_items(**where)
In offset mode the Operations UI and fleet grid poll GET /api/operator-queue. Each call now runs a COUNT over the same WHERE in addition to the page read and the flag counts. This is cheap at today's queue sizes, and total is the feature being asked for. If the queue table grows large, it would be worth checking that the count is served by the existing indexes.
Suggestion: no change needed now. Keep it in mind if the polling interval shrinks or the table grows.

Clean Categories

• SQL & data safety: every new predicate is built with SQLAlchemy expressions and bound parameters. The LIKE-free prefix match uses substr(...) != p.lower() over a constant tuple. No migrations, no DDL.
• Race conditions: the walk's sort key is fixed "as of" a watermark, and the pending-alert priorities are snapshotted per walk in Redis, so the key is shared across workers rather than held in worker-local state. I checked every pending→ended writer in db/operator_queue.py (cancel-batch, respond, cancel, batch cancel, expire): each stamps disposed_at=now in the same UPDATE and calls _note_commit_lag. The responded→acknowledged write does not stamp disposed_at and does not need to, because its row is already in the ended section. The one key-moving write on a pending row is _touch, which only reaches subject-keyed alerts, and the snapshot covers it.
• Auth boundaries: ?strict=true runs after the existing can_user_access_agent check. Agent-key narrowing only narrows, keys on mcp_scope, and leaves system and user keys untouched. agent_names is intersected with the caller's visible set. Cursor tokens carry no authority, and every page re-applies visibility. An agent scope with no agent identity gets a 403 (fails closed). The MCP checkAgentAccess gate is unchanged.
• Credential exposure: none. Log lines carry agent names and counts only.
• Enterprise disclosure in public docs: I ran the enterprise-docs-guard pattern over the added docs/ lines and got no hits.
• Error handling: Redis faults on the walk path map to a named 503. Malformed, mismatched or expired cursors return named 422/410 responses instead of a generic 500. A failed strict permissions read is surfaced as permissions_unavailable rather than a silent self-only answer.
• Enum / value completeness: there are no new status, type or priority values. The walk's priority rank covers critical, high, medium and low, with else_=4.
• Documentation staleness: architecture area files, feature flows, requirements and user docs are all updated in the diff.
• Product quality bar: the new fields are additive, offset mode keeps its ordering, an older backend shows up as null plus a warning rather than as "complete", and the tool description states the contract.

Low confidence (appendix)

• The walk's priority snapshot becomes an id IN (...) list sized by the number of pending platform alerts. That is bounded in practice, but nothing caps it explicitly. (Confidence: 4/10)
• As the PR body says, PostgreSQL behaviour (multi-byte ltrim, tie-order collation) is covered only by dialect compilation, and Redis only by fakeredis. UNVERIFIED live; noted rather than flagged.

Summary

• Critical: 0 — none found
• Informational: 2 — review recommended (I1 is a quick fix)
• Scope: clean

🤖 Generated with Claude Code

[dolho]

Approved — /review found no blocking findings; see review comment above.

[vybe]

merge-train (2026-10-07): changed the body's Refs to Fixes — validation found every acceptance criterion met (follow-ups are already filed separately), so the issue should close/promote on merge. Mechanical; nothing pushed to the branch.

[vybe]

merge-train (2026-10-07): merged as part of train #3329 (green). Before the squash, dev was merged into this branch because an earlier sibling had appended to the same files (tests/registry.json and/or docs/memory/feature-flows*.md). Both sides were kept, nothing else was changed.