fix(platform): close tasks/collab authz, orphan, and notification gaps - #3180
Draft
larryro wants to merge 8 commits into
Draft
fix(platform): close tasks/collab authz, orphan, and notification gaps#3180larryro wants to merge 8 commits into
larryro wants to merge 8 commits into
Conversation
POST /:taskId/agent-runs/:runId/cancel asserted write access on the URL's task, then cancelled by (runId, org) alone — a run id lifted from another project's task (one the caller may not even read) was cancelled and ledgered as a normal cancel. The route now resolves the run and answers the opaque 404 when it is not this task's, and `cancelAgentRunInTx` binds task_id inside its UPDATE predicate, so there is no unbound cancel door left: every lane that cancels a run holds its task. The in-tx door is what the task hard delete will use to cancel a subtree's live runs.
`assignTask` carried a TODO where the bulk bar already refused: a task whose agent run was mid-flight could be handed to someone else from the task sheet, the board card, or the list row, so the old agent kept driving (settle comments, the in_review park) a card that showed a new name, and "Run agent" answered already_running for the wrong agent. One `assigneeChanges` rule now decides what counts as a transfer for both doors; the single-card door refuses with TASK_HAS_LIVE_RUN (409) and the bulk bar keeps skipping. Every picker names the refusal in en/de/fr; the sheet's confirmed handoff still cancels the run first, then reassigns.
Reaching `in_review` is the request for review — the gate belongs to the state — but only the single-card doors minted it. The bulk bar, a non-workflow external close (`upsertTaskByExternalRef`), the agent's own `task_update_status` tool and the workflow `task.update_status` native parked cards at In review with no approval row: nobody was belled, nothing showed on the board, and the later leave to done recorded no approval. Every park now calls `requestTaskReview`. Lanes without a run key mint on the task's live run when one exists, so the settle's later park finds the same row instead of superseding it with a second bell; the workflow native mints an automation-keyed row. Idempotent, so a re-close of an already parked card heals a missing gate. The integration check also proves the run-cancel task binding from the previous commit end to end.
`deleteTask` removed the subtree, threads and approvals and stopped at a TODO: every deleted task leaked its attachment and output blobs into org storage for good (no sweep covered them), and a live agent run had its row FK-cascaded away mid-turn, leaving the sandbox turn executing with nowhere to land and no provenance entry. The delete now cancels the subtree's live agent runs through the ledgered in-tx door (the entry lands before the cascade; the turn host reaps the exec as an orphan) and bound automation runs through their terminal door, trashes the tasks' own unbound file rows for refs no surviving task lists, and enqueues the durable `knowledge.release_refs` job — the shared release seam deletes the bytes after commit and keeps any ref a document, chat thread or other file row still holds.
`task_reviewer_assigned` existed only in the dead Convex shim (`core/collab/notify_task_reviews.ts`); the live `updateTask` reviewer branch wrote the column and told nobody, so a person put on the hook missed the task's progress until the review request landed. Designating a new reviewer now subscribes them (reason 'reviewer') and writes the heads-up bell through the 0.5 collab writer — bell only, no email, pref gate skipped like the request, never for designating yourself — on the `notification` hint entity the app keys the bell on. A reviewer must be an active member of the org (`TASK_REVIEWER_INVALID` otherwise): the picker offers members only, so a miss is a stale or hand-built request.
`buildMentionDirectory` caught each listing failure (members, automations, agent instances), logged it and returned a partial directory — so on a transient error the comment posted fine while `@teammate` got no bell and `@automation`/`@agent` triggered nothing, silently. The "quiet refusal" contract covers permission misses, not infrastructure failures. A leg that cannot be listed now throws `MentionDirectoryError`; the task-comment door answers 503 `MENTION_DIRECTORY_UNAVAILABLE`, so the author sees a retryable failure instead of a comment that notified nobody.
`getMyAttentionSummary` fetched 100 pending task reviews org-wide in no order and filtered `requestedFor` in JS, so past 100 pending reviews in an org a person's own reviews fell outside the window nondeterministically — "waiting on me" flickered and could show zero while work waited on them. The reviewer filter and the project scope now run in SQL before the cap, newest gate first, with the exact total from a window count; the unread counts come from one GROUP BY aggregate instead of a capped page scan.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Seven verified medium findings in the task board's authorization, delete, review-gate and notification seams, from the backend deep-review campaign (
/tmp/tale-review/triage/mediums.md, modulestasks+collab). Base899fcc08a(has #3130 / #3157 / #3158 / #3164); every finding was re-verified live on that base before the fix. One commit per seam.Per-finding outcome
tasks/routes.tscancel by:runId)b97e14b74AGENT_RUN_NOT_FOUNDwhen it is not this task's;cancelAgentRunInTxbindstask_idinside the UPDATE predicate (no unbound cancel door left).deleteTaskTODO)954214d2ecancelRunInTx; the tasks' unboundfile_metadatarows are trashed for refs no surviving task lists and the durableknowledge.release_refsjob (#3140 seam) deletes the bytes after commit.0cb174b4eassigneeChangesrule forassignTaskandbulkUpdateTasks; the single door refusesTASK_HAS_LIVE_RUN(409), bulk keeps skipping; every picker names the refusal (en/de/fr). The comment-mention dispatcher only reachesassignTaskwith no live run (steer lane returns first), so it is unaffected.in_reviewwithout minting the review gateea58c2fa3requestTaskReviewnow runs from the bulk bar, the non-workflow external close, andagentUpdateTaskStatusTrusted(the agent'stask_update_statustool + the workflow native, which also parked without a gate). Lanes without a run key mint on the task's live run so the settle replay finds the same row; idempotent.67501dd96notifyTaskReviewerAssignedincollab/service.ts(bell only, pref gate skipped like the request, never for yourself) on thenotificationhint entity from #3164;updateTasksubscribes the designee (reasonreviewer) and refuses a non-member (TASK_REVIEWER_INVALID).6bfb0f7a2MentionDirectoryError; the task-comment door answers 503MENTION_DIRECTORY_UNAVAILABLEinstead of posting a comment that notified nobody.edd19cb38requestedFor+ project scope filtered in SQL before the cap,ORDER BY seq DESC, exact total via window count; unread counts from aGROUP BYaggregate instead of a capped page (badge now exact rather than capped at 100).No migration needed (0070/0071 untouched): the pending-review filter rides
approvals_org_status, and pending reviews are rare org-wide.Tests (red on base, green on branch)
Colocated vitest, each observed failing before its fix:
tasks/agent-runs.test.ts— the cancel bindstask_idin the guard; a refused cancel writes no ledger entry (2, red: nocancelAgentRunInTx).tasks/service.test.ts—assigneeChangestransfer rule + the 409 refusal (4).tasks/service.blobs.test.ts—collectTaskBlobRefsacross a subtree, de-dup, malformed rows (3, red: no export).collab/service.test.ts—notifyTaskReviewerAssignedwrites the row + bell hint with no pref gate, skips self (2, red); attention summary filtersrequestedForin SQL, exact counts (2, red on the JS filter).collab/mention-directory.test.ts— a failed member/automation listing rejects (2, red: "promise resolved instead of rejecting"), healthy directory still resolves.Integration (
backend/integration-check.ts, newcheckTasksCollabIntegrity, 8 probes): foreign-door cancel refused 404 + own door cancels once with one ledger row; bulk → In review mints the gate the leave-to-done resolves (completed/approve); external close parks WITH the gate; agent-tool park run-keyed + settle replay finds it (1 row), workflow park mints an automation row; hard delete → children gone, agent run ledgeredcancelled, automation runcancelled, deliverable blob gone from MinIO via the release job, file row reaped; reviewer designation → bell + subscription +notificationhint, self silent, non-member 400; mention directory leg down →MENTION_DIRECTORY_UNAVAILABLE/503/agents; the caller's review found behind 120 other pending reviews with the exact count, org-wide and project-scoped.Verification (observed)
bunx tsc --noEmit(platform): clean after every seam.bun run --filter @tale/platform lint: exit 0.bunx vitest --run --project server --project pii: 72,736 tests pass; 3app/routes/*.test.tsxfiles fail to LOAD withDenied ID …@fontsource/inter…woff2?url— the known worktree + symlinkednode_modulessymptom, also on base, unrelated.backend:integrationon fresh throwaway tale-db + MinIO (SANDBOX_LLM_GATEWAY_ADMIN_PASSWORDset), branch: 387/387 checks passed, all 8 new probes PASS.Cross-class discoveries (not fixed here)
core/collab/notify_task_reviews.tsand most ofcore/tasks/review_shared.tsare dead Convex-shim code still compiled by the platform tsconfig; onlyREVIEW_POLICY_REFUSAL_CODESis live. Cleanup candidate./api/app/collab/attentionsummary has no caller underapp/that I could find; its unread counts are now exact instead of capped at 100 — worth confirming the badge consumer (if any) renders large counts.TaskErrornow admits status 409 (previously 400|403|404).