fix(inbox): keep passive issue views out of Mine (#10581)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The inbox shows work that a board user owns or has joined > - Issue detail views update a per-user read receipt > - The Mine query treated that read receipt as user participation > - A passive view therefore added the issue to Mine > - This pull request separates passive reads from audited user mutations > - The benefit is a Mine inbox that reflects ownership and real participation ## Linked Issues or Issue Description **What happened?** Opening an issue detail marks the issue as read. The Mine query treated the read receipt as participation. The viewed issue then appeared in Mine even when the user did not change it. **Expected behavior** Viewing an issue can update unread state. A view alone must not add the issue to Mine. Issue creation, assignment, comments, and audited user mutations must add it. **Steps to reproduce** 1. Open an issue that you did not create and that is not assigned to you. 2. Do not comment or change the issue. 3. Open the Mine inbox. 4. Observe that the issue appears in Mine on the previous implementation. **Paperclip version or commit** `90ead239a8` **Deployment mode** Local dev from source. **Additional context** Related approach: #3421 changes Mine to an assignee-only filter. This change keeps participation-based Mine behavior and corrects the participation signal. ## What Changed - Use an explicit audited user-mutation allowlist for Mine participation, including comment cancellation. - Keep passive reads, previews, denied resource requests, and archive bookkeeping out of Mine participation. - Record manual routine reuse as an explicit audited inbox touch instead of a read receipt. - Keep that inbox bookkeeping from satisfying routine activity gates. - Add focused regression coverage for passive views, real mutations, comment cancellation, and manual routine runs. - Document the Mine participation contract. ## Verification - `pnpm exec vitest run server/src/__tests__/issues-service.test.ts -t "does not treat passive issue activity"` - `pnpm exec vitest run server/src/__tests__/routines-service.test.ts -t "touches a (coalesced|skipped active) routine issue|ignores inbox bookkeeping activity"` - `pnpm --filter @paperclipai/server typecheck` - GitHub latest-head CI: all required checks passed, including build, typecheck, server suites, and e2e shards. ## Risks - Low risk. The change affects only the server query that defines user participation in Mine and the manual routine touch signal. - Historical audited user mutations can now qualify an issue for Mine. Passive read and archive actions remain excluded. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI Codex, GPT-5 family. The runtime did not expose the exact deployment ID or context window. Agentic reasoning, tool use, and code execution were enabled. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Dotta
GitHub
parent
e4b0152ca3
commit
14d4db6330
|
|
@ -818,6 +818,7 @@ Core authorization follows these rules:
|
|||
- An agent targeting any user other than its resolved responsible user requires an explicit `inbox:manage` grant. Grants may be unscoped or constrained by `scope.userIds`.
|
||||
- Archive and unarchive operations are company-scoped, reversible, and activity logged with actor, agent, run, target user, target-resolution source, and policy mode.
|
||||
- New qualifying issue activity may invalidate an archive so the item resurfaces; archival is not a substitute for resolving or closing work.
|
||||
- Viewing an issue may update its per-user read receipt, but read receipts alone do not enroll the issue in Mine. Mine participation begins with a user-authored comment, issue creation/assignment, or another audited user mutation; explicit product actions such as manually running a routine may record an audited inbox touch.
|
||||
|
||||
Ownership split:
|
||||
|
||||
|
|
|
|||
|
|
@ -18,6 +18,7 @@ import {
|
|||
issueInboxArchives,
|
||||
issueDocuments,
|
||||
issuePlanDecompositions,
|
||||
issueReadStates,
|
||||
issueRelations,
|
||||
issueThreadInteractions,
|
||||
issues,
|
||||
|
|
@ -308,6 +309,7 @@ describeEmbeddedPostgres("issueService.list participantAgentId", () => {
|
|||
await db.delete(issueRelations);
|
||||
await db.delete(issueDocuments);
|
||||
await db.delete(issueInboxArchives);
|
||||
await db.delete(issueReadStates);
|
||||
await db.delete(activityLog);
|
||||
await db.delete(issues);
|
||||
await db.delete(documents);
|
||||
|
|
@ -336,6 +338,70 @@ describeEmbeddedPostgres("issueService.list participantAgentId", () => {
|
|||
return companyId;
|
||||
}
|
||||
|
||||
it("does not treat passive issue activity as touching it, but includes real user mutations", async () => {
|
||||
const companyId = await seedAssignableAgentCompany();
|
||||
const issueId = randomUUID();
|
||||
const userId = "board-user";
|
||||
|
||||
await db.insert(issues).values({
|
||||
id: issueId,
|
||||
companyId,
|
||||
title: "Issue viewed without participation",
|
||||
status: "todo",
|
||||
priority: "medium",
|
||||
});
|
||||
await svc.markRead(companyId, issueId, userId);
|
||||
await db.insert(activityLog).values([
|
||||
{
|
||||
companyId,
|
||||
actorType: "user",
|
||||
actorId: userId,
|
||||
action: "issue.read_marked",
|
||||
entityType: "issue",
|
||||
entityId: issueId,
|
||||
},
|
||||
{
|
||||
companyId,
|
||||
actorType: "user",
|
||||
actorId: userId,
|
||||
action: "issue.file_resource_content_read",
|
||||
entityType: "issue",
|
||||
entityId: issueId,
|
||||
},
|
||||
{
|
||||
companyId,
|
||||
actorType: "user",
|
||||
actorId: userId,
|
||||
action: "issue.file_resource_download_denied",
|
||||
entityType: "issue",
|
||||
entityId: issueId,
|
||||
},
|
||||
{
|
||||
companyId,
|
||||
actorType: "user",
|
||||
actorId: userId,
|
||||
action: "issue.tree_control_previewed",
|
||||
entityType: "issue",
|
||||
entityId: issueId,
|
||||
},
|
||||
]);
|
||||
|
||||
await expect(svc.list(companyId, { touchedByUserId: userId })).resolves.toEqual([]);
|
||||
|
||||
await db.insert(activityLog).values({
|
||||
companyId,
|
||||
actorType: "user",
|
||||
actorId: userId,
|
||||
action: "issue.comment_cancelled",
|
||||
entityType: "issue",
|
||||
entityId: issueId,
|
||||
});
|
||||
|
||||
await expect(svc.list(companyId, { touchedByUserId: userId })).resolves.toEqual([
|
||||
expect.objectContaining({ id: issueId }),
|
||||
]);
|
||||
});
|
||||
|
||||
function agentRow(companyId: string, input: {
|
||||
id: string;
|
||||
name: string;
|
||||
|
|
|
|||
|
|
@ -16,7 +16,6 @@ import {
|
|||
heartbeatRuns,
|
||||
instanceSettings,
|
||||
issueInboxArchives,
|
||||
issueReadStates,
|
||||
issues,
|
||||
projectWorkspaces,
|
||||
projects,
|
||||
|
|
@ -64,7 +63,6 @@ describeEmbeddedPostgres("routine service live-execution coalescing", () => {
|
|||
}
|
||||
await db.delete(activityLog);
|
||||
await db.delete(issueInboxArchives);
|
||||
await db.delete(issueReadStates);
|
||||
await db.delete(secretAccessEvents);
|
||||
await db.delete(companySecretBindings);
|
||||
await db.delete(routineRuns);
|
||||
|
|
@ -429,7 +427,7 @@ describeEmbeddedPostgres("routine service live-execution coalescing", () => {
|
|||
expect(agentId).not.toBe(otherAgentId);
|
||||
});
|
||||
|
||||
it("fires for a human comment and ignores pure-read activity", async () => {
|
||||
it("fires for a human comment and ignores inbox bookkeeping activity", async () => {
|
||||
const { companyId, projectId, routine, svc } = await seedFixture();
|
||||
const windowStart = new Date(Date.now() - 60_000);
|
||||
const now = new Date();
|
||||
|
|
@ -455,6 +453,15 @@ describeEmbeddedPostgres("routine service live-execution coalescing", () => {
|
|||
entityId: issueId,
|
||||
createdAt: new Date(windowStart.getTime() + 2_000),
|
||||
},
|
||||
{
|
||||
companyId,
|
||||
actorType: "user",
|
||||
actorId: "user-1",
|
||||
action: "issue.inbox_touched",
|
||||
entityType: "issue",
|
||||
entityId: issueId,
|
||||
createdAt: new Date(windowStart.getTime() + 3_000),
|
||||
},
|
||||
]);
|
||||
|
||||
await expect(svc.evaluateActivityGate(routine, now)).resolves.toMatchObject({
|
||||
|
|
@ -482,6 +489,15 @@ describeEmbeddedPostgres("routine service live-execution coalescing", () => {
|
|||
entityId: issueId,
|
||||
createdAt: new Date(windowStart.getTime() + 2_000),
|
||||
},
|
||||
{
|
||||
companyId,
|
||||
actorType: "user",
|
||||
actorId: "user-1",
|
||||
action: "issue.inbox_touched",
|
||||
entityType: "issue",
|
||||
entityId: issueId,
|
||||
createdAt: new Date(windowStart.getTime() + 3_000),
|
||||
},
|
||||
]);
|
||||
|
||||
await expect(svc.evaluateActivityGate(routine, now)).resolves.toMatchObject({ fire: false });
|
||||
|
|
@ -1240,12 +1256,15 @@ describeEmbeddedPostgres("routine service live-execution coalescing", () => {
|
|||
db.select().from(issueInboxArchives).where(eq(issueInboxArchives.issueId, previousIssue.id)),
|
||||
).resolves.toHaveLength(0);
|
||||
await expect(
|
||||
db.select().from(issueReadStates).where(eq(issueReadStates.issueId, previousIssue.id)),
|
||||
db.select().from(activityLog).where(eq(activityLog.entityId, previousIssue.id)),
|
||||
).resolves.toEqual([
|
||||
expect.objectContaining({
|
||||
companyId,
|
||||
issueId: previousIssue.id,
|
||||
userId,
|
||||
actorType: "user",
|
||||
actorId: userId,
|
||||
action: "issue.inbox_touched",
|
||||
entityType: "issue",
|
||||
entityId: previousIssue.id,
|
||||
}),
|
||||
]);
|
||||
|
||||
|
|
@ -1323,12 +1342,15 @@ describeEmbeddedPostgres("routine service live-execution coalescing", () => {
|
|||
db.select().from(issueInboxArchives).where(eq(issueInboxArchives.issueId, previousIssue.id)),
|
||||
).resolves.toHaveLength(0);
|
||||
await expect(
|
||||
db.select().from(issueReadStates).where(eq(issueReadStates.issueId, previousIssue.id)),
|
||||
db.select().from(activityLog).where(eq(activityLog.entityId, previousIssue.id)),
|
||||
).resolves.toEqual([
|
||||
expect.objectContaining({
|
||||
companyId,
|
||||
issueId: previousIssue.id,
|
||||
userId,
|
||||
actorType: "user",
|
||||
actorId: userId,
|
||||
action: "issue.inbox_touched",
|
||||
entityType: "issue",
|
||||
entityId: previousIssue.id,
|
||||
}),
|
||||
]);
|
||||
|
||||
|
|
|
|||
|
|
@ -1317,6 +1317,64 @@ async function getWorkspaceInheritanceIssue(
|
|||
return issue;
|
||||
}
|
||||
|
||||
// Mine participation fails closed. Add new user-authored issue mutation actions
|
||||
// here instead of admitting every issue activity, because reads, previews, and
|
||||
// denied resource requests are audited too.
|
||||
const ISSUE_USER_PARTICIPATION_ACTIVITY_ACTIONS = [
|
||||
"issue.accepted_plan_decomposition_updated",
|
||||
"issue.admin_force_release",
|
||||
"issue.approval_linked",
|
||||
"issue.approval_unlinked",
|
||||
"issue.approvers_updated",
|
||||
"issue.assigned",
|
||||
"issue.attachment_added",
|
||||
"issue.attachment_removed",
|
||||
"issue.blockers.updated",
|
||||
"issue.blockers_updated",
|
||||
"issue.checked_out",
|
||||
"issue.checkout",
|
||||
"issue.child_created",
|
||||
"issue.comment_cancelled",
|
||||
"issue.document_annotation_comment_added",
|
||||
"issue.document_annotation_remapped",
|
||||
"issue.document_annotation_thread_created",
|
||||
"issue.document_annotation_thread_resolved",
|
||||
"issue.document_deleted",
|
||||
"issue.document_locked",
|
||||
"issue.document_restored",
|
||||
"issue.document_unlocked",
|
||||
"issue.document_updated",
|
||||
"issue.document_upserted",
|
||||
"issue.feedback_vote_saved",
|
||||
"issue.inbox_touched",
|
||||
"issue.low_trust_output_promoted",
|
||||
"issue.monitor_cleared",
|
||||
"issue.monitor_scheduled",
|
||||
"issue.recovery_action_resolved",
|
||||
"issue.relations.updated",
|
||||
"issue.released",
|
||||
"issue.reviewers_updated",
|
||||
"issue.scheduled_retry_retry_now",
|
||||
"issue.successful_run_handoff_resolved",
|
||||
"issue.task_watchdog_fingerprint_reviewed",
|
||||
"issue.thread_interaction_accepted",
|
||||
"issue.thread_interaction_answered",
|
||||
"issue.thread_interaction_cancelled",
|
||||
"issue.thread_interaction_created",
|
||||
"issue.thread_interaction_item_verdicts_submitted",
|
||||
"issue.thread_interaction_withdrawn",
|
||||
"issue.tree_cancel_status_updated",
|
||||
"issue.tree_hold_created",
|
||||
"issue.tree_hold_released",
|
||||
"issue.tree_restore_status_updated",
|
||||
"issue.updated",
|
||||
"issue.watchdog_created",
|
||||
"issue.watchdog_removed",
|
||||
"issue.work_product_created",
|
||||
"issue.work_product_deleted",
|
||||
"issue.work_product_updated",
|
||||
] as const;
|
||||
|
||||
function touchedByUserCondition(companyId: string, userId: string) {
|
||||
return sql<boolean>`
|
||||
(
|
||||
|
|
@ -1324,10 +1382,16 @@ function touchedByUserCondition(companyId: string, userId: string) {
|
|||
OR ${issues.assigneeUserId} = ${userId}
|
||||
OR EXISTS (
|
||||
SELECT 1
|
||||
FROM ${issueReadStates}
|
||||
WHERE ${issueReadStates.issueId} = ${issues.id}
|
||||
AND ${issueReadStates.companyId} = ${companyId}
|
||||
AND ${issueReadStates.userId} = ${userId}
|
||||
FROM ${activityLog}
|
||||
WHERE ${activityLog.entityType} = 'issue'
|
||||
AND ${activityLog.entityId} = ${issues.id}::text
|
||||
AND ${activityLog.companyId} = ${companyId}
|
||||
AND ${activityLog.actorType} = 'user'
|
||||
AND ${activityLog.actorId} = ${userId}
|
||||
AND ${activityLog.action} IN (${sql.join(
|
||||
ISSUE_USER_PARTICIPATION_ACTIVITY_ACTIONS.map((action) => sql`${action}`),
|
||||
sql`, `,
|
||||
)})
|
||||
)
|
||||
OR EXISTS (
|
||||
SELECT 1
|
||||
|
|
|
|||
|
|
@ -16,7 +16,6 @@ import {
|
|||
goals,
|
||||
heartbeatRuns,
|
||||
issueInboxArchives,
|
||||
issueReadStates,
|
||||
issues,
|
||||
pluginManagedResources,
|
||||
plugins,
|
||||
|
|
@ -88,6 +87,7 @@ const ACTIVITY_GATE_IGNORED_ACTIONS = [
|
|||
"issue.read_unmarked",
|
||||
"issue.inbox_archived",
|
||||
"issue.inbox_unarchived",
|
||||
"issue.inbox_touched",
|
||||
];
|
||||
const WEEKDAY_INDEX: Record<string, number> = {
|
||||
Sun: 0,
|
||||
|
|
@ -1583,22 +1583,17 @@ export function routineService(
|
|||
touchedAt: Date;
|
||||
},
|
||||
) {
|
||||
await executor
|
||||
.insert(issueReadStates)
|
||||
.values({
|
||||
companyId: input.companyId,
|
||||
issueId: input.issueId,
|
||||
userId: input.userId,
|
||||
lastReadAt: input.touchedAt,
|
||||
updatedAt: input.touchedAt,
|
||||
})
|
||||
.onConflictDoUpdate({
|
||||
target: [issueReadStates.companyId, issueReadStates.issueId, issueReadStates.userId],
|
||||
set: {
|
||||
lastReadAt: input.touchedAt,
|
||||
updatedAt: input.touchedAt,
|
||||
},
|
||||
});
|
||||
await executor.insert(activityLog).values({
|
||||
companyId: input.companyId,
|
||||
actorType: "user",
|
||||
actorId: input.userId,
|
||||
action: "issue.inbox_touched",
|
||||
entityType: "issue",
|
||||
entityId: input.issueId,
|
||||
responsibleUserId: input.userId,
|
||||
details: { source: "manual_routine_run" },
|
||||
createdAt: input.touchedAt,
|
||||
});
|
||||
|
||||
await executor
|
||||
.delete(issueInboxArchives)
|
||||
|
|
|
|||
Loading…
Reference in New Issue