feat(requests): P1 — unified requests/inbox data model + endpoints (RFC) #2525
Reference in New Issue
Block a user
Delete Branch "feat/unified-requests-inbox-p1"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Phase 1 — unified requests / inbox (RFC)
Implements P1 of the unified-requests-inbox RFC (
docs/design/rfc-unified-requests-inbox.md, added here): a singlerequestssubsystem that generalizesuser_tasks(agent→user worklist asks) andapproval_requests(the destructive-action gate) into one inbox keyed bykind ∈ {task, approval}, where requester and recipient may each be a user OR an agent. Responding is asynchronous — the requester never blocks; aREQUEST_RESPONDEDevent signals it to pick the answer up on its next tick.Schema —
migrations/20260610120000_requests.{up,down}.sql(idempotent)requests:kind,requester_{type,id},org_id,recipient_{type,id},title,detail,status(pending|info_requested|done|rejected|approved|cancelled),responder_{type,id},priority,created/updated/responded_at.recipient_id/requester_idare plain TEXT with no FK (a party can be a user, not aworkspacesrow). Indexes for inbox(recipient_type, recipient_id, status, created_at DESC), org-pending(org_id, status, created_at DESC), outgoing(requester_type, requester_id, created_at DESC).request_messages: the More-Info / "chat about this" thread (request_idFK →requestsON DELETE CASCADE), indexed(request_id, created_at).ON CONFLICT (id) DO NOTHING) copies historicaluser_tasks(kind=task;dismissed→rejected) andapproval_requests(kind=approval;denied→rejected,escalated→pending) into the unified inbox so the tabs show pre-cutover items. EverythingCREATE … IF NOT EXISTS— no bareALTER ADD CONSTRAINT— so the re-apply-on-boot runner can't crash-loop.Store —
internal/handlers/request_store.goRequestStoremirrorsUserTaskStore: built per-request over the globaldb.DB, takesevents.EventEmitterfor testability, sentinel errors (ErrRequestNotFound,ErrInvalidRequestAction, …). Methods:Create/Get/Messages/ListInbox/ListOutgoing/ListPendingForOrg(LEFT JOINworkspacesfor the agent name) /Respond(validates action↔kind: approval→approved|rejected, task→done|rejected) /RequestInfo/AddMessage(flipsinfo_requestedwhen the recipient asks back) /Cancel. Mutations broadcastREQUEST_CREATED/REQUEST_RESPONDED/REQUEST_MESSAGEanchored on the agent party so the canvas/inbox is signalled — the async signal-back.Handler + routes —
internal/handlers/requests.go,internal/router/router.goMirrors the approvals/user-tasks auth split:
POST /workspaces/:id/requests,GET …/requests(outgoing pickup),GET …/requests/inbox, plus agent-side…/requests/:requestId/{get,respond,messages,cancel}.GET /requests/pending?kind=task|approval(the two tabs), and/requests/:requestId/{get,respond,messages,cancel}(responder defaults touser).org_idanchor is resolved viaorgRootID(org_scope.go) — theworkspacestable has noorg_idcolumn.Events —
internal/events/types.goAdds
REQUEST_CREATED,REQUEST_RESPONDED,REQUEST_MESSAGEto the taxonomy +AllEventTypes; the drift snapshot (types_test.go) is updated. The canvas TS mirror is a later phase and is not touched here.Tests —
internal/handlers/requests_test.go20 cases via the existing
sqlmockharness: create (task + approval, agent recipient, missing title, invalid kind), inbox vs outgoing listing, get + thread, respond (valid approval/task + invalid action-for-kind + not-found), More-Info message →info_requested(recipient flips, requester doesn't), cancel (+ not-found), org pending list (+ kind filter + invalid kind), recipient routing (user vs agent). All green; fullinternal/handlers+internal/eventssuites pass.Scope / phasing
This is P1 only. Not included (later phases): P2 MCP tools +
create_approval/decide_approvalshims, P3 canvas Tasks/Approvals tabs UI + More-Info thread + WS live, P4 idle-nudge worker + user-pending notifications.Notes / deviations
docs/design/(alongsiderfc-user-tasks.md), not underworkspace-server/docs/design/, to match the existing RFC location.🤖 Generated with Claude Code
Implements Phase 1 of the unified-requests-inbox RFC: a single `requests` subsystem that generalizes `user_tasks` (agent→user worklist asks) and `approval_requests` (the destructive-action gate) into one inbox keyed by `kind ∈ {task, approval}`, where requester and recipient may each be a user OR an agent. Responding is asynchronous — the requester never blocks; a REQUEST_RESPONDED event signals it to pick the answer up on its next tick. Schema (migrations/20260610120000_requests.{up,down}.sql, idempotent): - `requests`: kind, requester_{type,id}, org_id, recipient_{type,id}, title, detail, status (pending|info_requested|done|rejected|approved|cancelled), responder_{type,id}, priority, created/updated/responded_at. recipient_id / requester_id are plain TEXT with NO FK (a party may be a user, not a workspaces row). Indexes for inbox, org-pending, and outgoing reads. - `request_messages`: the More-Info / "chat about this" thread (FK → requests, ON DELETE CASCADE), indexed by (request_id, created_at). - Idempotent backfill (ON CONFLICT (id) DO NOTHING) copies historical user_tasks (kind=task; dismissed→rejected) and approval_requests (kind=approval; denied→rejected, escalated→pending) into the unified inbox so the tabs show pre-cutover items. Store (internal/handlers/request_store.go): RequestStore mirrors UserTaskStore — per-request over global db.DB, events.EventEmitter for testability, sentinel errors. Create / Get / Messages / ListInbox / ListOutgoing / ListPendingForOrg (LEFT JOIN workspaces for the agent name) / Respond (validates action↔kind: approval→approved|rejected, task→done|rejected) / RequestInfo / AddMessage (flips info_requested when the recipient asks back) / Cancel. Mutations broadcast REQUEST_CREATED / REQUEST_RESPONDED / REQUEST_MESSAGE anchored on the agent party so the canvas/inbox is signalled. Handler (internal/handlers/requests.go) + routes (internal/router/router.go), mirroring the approvals/user-tasks auth split: - wsAuth (workspace token): POST /workspaces/:id/requests, GET .../requests (outgoing), GET .../requests/inbox, plus agent-side .../requests/:requestId/{get,respond,messages,cancel}. - AdminAuth (canvas user): GET /requests/pending?kind=task|approval (the tabs), and /requests/:requestId/{get,respond,messages,cancel}. The org_id anchor is resolved via orgRootID (org_scope.go) — the workspaces table has no org_id column. Events (internal/events/types.go): adds REQUEST_CREATED, REQUEST_RESPONDED, REQUEST_MESSAGE to the taxonomy + AllEventTypes; drift snapshot (types_test.go) updated. Canvas TS mirror is a later phase (not touched here). Tests (internal/handlers/requests_test.go): 20 cases via the existing sqlmock harness — create (task + approval, agent recipient), inbox vs outgoing, get+thread, respond (valid + invalid action-for-kind + not-found), More-Info message → info_requested (recipient flips, requester doesn't), cancel, org pending list + kind filter, recipient routing. This is P1 of the unified-requests RFC. Follow-ons: P2 MCP tools + approval shims, P3 canvas Tasks/Approvals tabs UI, P4 idle-nudge worker. Not included here. RFC: docs/design/rfc-unified-requests-inbox.md Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>Security 5-axis — REQUEST_CHANGES (head
59e5b20724). feat(requests): P1 unified requests/inbox. The store layer + migration are clean; one genuine authorization gap on the agent action paths blocks (privilege/approval-integrity).BLOCKER — missing participant-binding on the agent action verbs (self-approval / cross-respond):
Respond(handlers/requests.go) uses onlyc.Param("requestId")+ bodyAction/ResponderType/ResponderID→store.Respond(id, action, responderType, responderID), whose SQL isUPDATE requests … WHERE id=$4 AND status IN (pending,info_requested)— no recipient check. On the agent routewsAuth POST /workspaces/:id/requests/:requestId/respond, the handler never compares the authenticated:idto the request’srecipient_id. Consequences:{action:"approve"}on its own request → it flips toapproved. The approval gate is satisfiable by the party it’s meant to gate.responder_idis body-supplied and unvalidated → the recorded responder is spoofable.Same participant-binding gap on
Cancel(no requester check — anyone cancels any request),AddMessage(body author unvalidated), andGet(no participant scope → cross-tenant read of title/detail/thread).Minimal fix: on the AGENT (wsAuth) paths, bind to the authenticated
:id—Respond/AddMessagerequirerequest.recipient_id == :id(recipient_type=agent);Cancelrequiresrequest.requester_id == :id;Getrequires caller ∈ {requester, recipient}; derive the actor identity from:id, not the body, for the agent path. The admin/canvas path (AdminAuth) acting as the human approver is fine as-is. (Accepting body-supplied identity for the audit field in P1 is OK — but authorization must not depend on it.)What’s sound (no change needed):
fmt.Sprintfonly builds placeholder NUMBERS ($%d), never interpolates values. No injection. ✓requests/request_messageswith CHECK-enum constraints, FKON DELETE CASCADE, sensible indexes; backfill fromuser_tasksisINSERT…SELECT…ON CONFLICT DO NOTHING(idempotent, additive);IF NOT EXISTSup, cleanDROPdown. Data-safe. ✓:id(not body) — correctly unspoofable. ✓ org_id resolved server-side via orgRootID (best-effort, NULL-safe).Net: clean data layer + correct Create; the agent action paths need recipient/requester binding before the approval workflow is safe. Add the binding → I convert to APPROVE. Author devops-engineer ≠ me; security 1st lane.
⚠️ ORDERING: P3 #2527 (canvas) + P4 #2526 (nudge sweeper) were just merged to main, but they DEPEND on THIS PR's
requeststable +/requestsendpoints. P1 is CI-green + mergeable. Please prioritize the 2-genuine review here so main isn't left with dependents referencing a non-existent table/endpoints. This is the foundational PR the others build on.agent-dev-a referenced this pull request2026-06-10 15:23:47 +00:00