Skip to content

fix(operator-queue): scope request-id uniqueness per agent (#1631)#1684

Merged
vybe merged 4 commits into
devfrom
feature/1631-operator-queue-id-collision
Jul 17, 2026
Merged

fix(operator-queue): scope request-id uniqueness per agent (#1631)#1684
vybe merged 4 commits into
devfrom
feature/1631-operator-queue-id-collision

Conversation

@obasilakis

Copy link
Copy Markdown
Contributor

Summary

operator_queue.id was a fleet-wide TEXT PRIMARY KEY populated with an agent-authored id read from each agent's ~/.trinity/operator-queue.json. Two agents choosing the same id collided on the PK and the second agent's request was silently never created — never shown to an operator, never answered, never flipped terminal in the agent's file (write-back is agent-scoped), so the agent waited forever with zero log signal. The #1402 execution-id-derived-id contract is prompt-level only; a template with a hardcoded id deployed twice still trips it.

The id was doing two jobs at once — the platform's global row handle and the agent's private correlation key. This splits them:

  • id → platform-minted uuid.uuid4().hex. Globally unique by construction. REST/MCP/frontend lookups unchanged (they always treated id as opaque).
  • request_id (new nullable column) → the agent's string, with UNIQUE(agent_name, request_id). create_item targets on_conflict_do_nothing at that index, so a same-agent re-insert stays idempotent and re-reads/returns the surviving row's uuid (preserves _create_park_item's durability contract).

Two cross-agent defects in the same path are fixed as a consequence:

New sync-loop guard: agent-authored ids using reserved platform prefixes (poison-/sync-failing-/git-bloat-/cb-dormant-/skill-not-found-) are rejected (log-once, bounded) so an agent can't pre-claim and hijack the platform's own alert ids.

Changes

  • Schema (dual-track, all four files): db/schema.py, db/tables.py, db/migrations.py (operator_queue_request_id, additive: add column, backfill request_id = id, add unique index), Alembic 0025_operator_queue_request_id (down_revision 0024).
  • db/operator_queue.py: uuid mint + conflict re-read in create_item; agent-scoped item_exists / mark_acknowledged (returns the row uuid for the WS event); dead get_pending_item_ids removed.
  • database.py: two facade signatures.
  • services/operator_queue_service.py: agent-scoped exists/ack, (agent, id)-keyed quarantine map, request_id-keyed write-back, reserved-prefix rejection.
  • components/operator/QueueItemDetail.vue: reference span shows request_id || id.
  • Docs: operating-room.md, architecture.md, requirements/scheduling.md.

Test Plan

  • New tests/unit/test_1631_operator_queue_agent_scoped_ids.py — two agents same request-id both persist; same-agent re-insert idempotent (returns existing uuid); item_exists and mark_acknowledged agent-scoped; reserved-prefix rejection.
  • Updated test_1426, test_1525, test_1081_lease_reaper for the new signatures.
  • 64 passed across the operator-queue unit surface + test_schema_parity + test_alembic_revision_id_length.
  • Independent adversarial review: no critical/high findings; conflict re-read valid on SQLite+Postgres; unique-index migration safe on populated data (old id was globally unique → backfill can't create dup pairs).

Note: tests/test_operator_queue.py is a live-server API test with a pre-existing sys.path isolation bug (reproduces on clean dev) — unrelated to this change.

Fixes #1631

obasilakis and others added 4 commits July 17, 2026 20:44
…e global PK (#1631)

`operator_queue.id` was a fleet-wide TEXT PRIMARY KEY populated with an
agent-authored id read from each agent's ~/.trinity/operator-queue.json. Two
agents choosing the same id collided on the PK and the second agent's request
was silently never created — never shown to an operator, never answered, never
flipped terminal in the agent's file (write-back is agent-scoped), so the agent
waited forever with zero log signal. The #1402 execution-id-derived-id contract
is prompt-level only; a template with a hardcoded id deployed twice still hits it.

Split the id's two jobs:
- `id` is now a platform-minted uuid4 hex — the global row handle. REST, MCP,
  and frontend lookups are unchanged (they always treated id as opaque).
- new nullable `request_id` column holds the agent's correlation string, with
  UNIQUE(agent_name, request_id). create_item targets on_conflict_do_nothing at
  that index, so a same-agent re-insert stays idempotent and re-reads/returns the
  surviving row's uuid (preserves _create_park_item's durability contract).

Also fixes two cross-agent defects in the same path:
- mark_acknowledged and item_exists are now (agent_name, request_id)-scoped, so
  agent B acknowledging its id can no longer flip agent A's row.
- the #1525 _create_failures quarantine map is keyed by (agent_name, req_id).

New sync-loop guard: agent-authored ids using reserved platform prefixes
(poison-/sync-failing-/git-bloat-/cb-dormant-/skill-not-found-) are rejected
(log-once, bounded) so an agent can't pre-claim and hijack the platform's own
alert ids.

Migration is additive (add column, backfill request_id = id, add unique index) —
dual-track: db/migrations.py `operator_queue_request_id` + Alembic 0025.

Fixes #1631
…hain Alembic revision to 0026 after #1676 took 0025

Both PRs chained an 0025_* revision onto 0024; #1676 merged first, so this PR's revision becomes 0026_operator_queue_request_id with down_revision 0025_agent_ownership_display_label. SQLite migration list keeps both entries in dev order.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ine adjacent service-catalog rows (#1616 cleanup row + #1631 operator-queue row)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@vybe vybe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validated via /validate-pr. Correct split of the two jobs operator_queue.id was doing: platform-minted uuid as the global handle, agent string in request_id with UNIQUE(agent_name, request_id); insert+re-read in one txn honours the surviving-row contract; exists/ack/quarantine/write-back all agent-scoped (closes the cross-agent ack flip); reserved-prefix hijack guard log-once + bounded. Dual-track migration complete — SQLite additive add+backfill+index, Alembic re-chained to 0026 on top of #1676's 0025 (single head verified locally + pg-migrations green). Adjacent architecture.md service-row conflict resolved combining #1616 + #1631 rows; 76 tests green on the merged tree. All checks green.

@vybe
vybe merged commit 94d854a into dev Jul 17, 2026
24 checks passed
vybe pushed a commit that referenced this pull request Jul 18, 2026
…es (#1631 follow-up) (#1688)

Fast-follow to PR #1684 (merged): the /cso --diff audit of the #1631 change
found one LOW — the reserved-prefix hijack guard was exact-match, so an agent
could author a lookalike (` Poison-x`, uppercase/padded) platform-styled
operator alert. Not a hijack (an independent refutation proved collision/
suppression is impossible — the only colliding string necessarily matches the
prefix and is rejected), but an operator-phishing surface.

Normalize (strip + lower) before the prefix check so a lookalike can't slip
past — the platform mints these prefixes lowercase and unpadded, so a
normalized match can only be impersonation. Covered by a new fold-case test.
Also lands the CSO audit report under docs/security-reports/.

This stranded on the #1684 branch when that PR squash-merged mid-audit; landing
it here on its own. Refs #1631.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants