Skip to content

feat: dismiss failed runs from inbox#765

Open
russellju16-afk wants to merge 1 commit intopaperclipai:masterfrom
russellju16-afk:feat/inbox-dismiss-failed-runs
Open

feat: dismiss failed runs from inbox#765
russellju16-afk wants to merge 1 commit intopaperclipai:masterfrom
russellju16-afk:feat/inbox-dismiss-failed-runs

Conversation

@russellju16-afk
Copy link
Copy Markdown

Summary

  • Adds dismissed_at column to heartbeat_runs (migration 0026) and filters dismissed runs from sidebar badge counts
  • Adds POST /heartbeat-runs/:runId/dismiss board endpoint and dismissRun service method
  • Inbox: dismiss button on FailedRunCard, optimistic local dismiss, new/all tabs with category filters and touched-issues section

Changes

DB / Shared

  • packages/db/src/schema/heartbeat_runs.ts: add dismissedAt column
  • Migration 0026_awesome_gateway.sql: ALTER TABLE heartbeat_runs ADD COLUMN dismissed_at TIMESTAMP WITH TIME ZONE
  • packages/shared/src/types/heartbeat.ts: add dismissedAt: Date | null to HeartbeatRun

Server

  • server/src/services/heartbeat.ts: dismissRun(runId) sets dismissedAt = now()
  • server/src/routes/agents.ts: POST /heartbeat-runs/:runId/dismiss
  • server/src/services/sidebar-badges.ts: filter isNull(heartbeatRuns.dismissedAt) so dismissed runs don't count toward the inbox badge
  • server/src/adapters/process/execute.ts: inject PAPERCLIP_API_KEY from authToken in process adapter env

UI

  • ui/src/api/heartbeats.ts: add heartbeatsApi.dismiss(runId)
  • ui/src/pages/Inbox.tsx: dismiss button + mutation on FailedRunCard; filter runs by !r.dismissedAt; new/all tabs; category filter dropdown; touched-issues section; aggregate agent-error alert banner

CLI

  • Add disableSignUp: false to defaultConfig() in configure command

Test plan

  • Start dev server, trigger a failed run, confirm it appears in Inbox
  • Click dismiss on a failed run card — card disappears immediately (optimistic) and sidebar badge decrements
  • Refresh page — dismissed run no longer appears (server-side filter)
  • Switch to "All" tab — category filter dropdown appears with correct sections
  • Typecheck, tests, and build all pass: pnpm -r typecheck && pnpm test:run && pnpm build

🤖 Generated with Claude Code

Adds the ability to dismiss failed heartbeat run alerts from the Inbox,
removing them from the attention queue and the sidebar badge count.

- Add `dismissed_at` column to `heartbeat_runs` (migration 0026)
- Add `dismissRun` service method that sets `dismissedAt`
- Add `POST /heartbeat-runs/:runId/dismiss` board endpoint
- Filter dismissed runs out of sidebar badge failed-run count
- Add `heartbeatsApi.dismiss()` client call
- Inbox: dismiss button on FailedRunCard, optimistic local dismiss,
  filter out runs with `dismissedAt` set
- Inbox: new/all tabs, category filters, touched-issues section,
  aggregate agent-error alert
- Process adapter: inject `PAPERCLIP_API_KEY` from auth token
- CLI: add `disableSignUp: false` to defaultConfig
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 13, 2026

Greptile Summary

This PR implements a "dismiss failed run" feature end-to-end: a new dismissed_at DB column (migration 0026), a POST /heartbeat-runs/:runId/dismiss server route, a dismissRun service method, updated sidebar badge filtering, and inbox UI changes (dismiss button, optimistic local removal, new/all tabs, category filter, touched-issues section). The overall architecture is sound and the migration is safe, but there are two correctness issues to address before merging:

  • Missing company-level access check on the dismiss routePOST /heartbeat-runs/:runId/dismiss only calls assertBoard(req) and never verifies the caller belongs to the same company as the run. In a multi-tenant deployment this allows cross-tenant dismissal. Every comparable read/write endpoint (e.g. the events route immediately below) correctly calls assertCompanyAccess(req, run.companyId).
  • Badge count / inbox display mismatch — The sidebar badge SQL uses isNull(dismissedAt) before DISTINCT ON, so it finds the latest non-dismissed run per agent. getLatestFailedRunsByAgent on the client picks the absolute latest run per agent (including dismissed ones) and then filters by !r.dismissedAt. If an agent's newest run is a dismissed failure and the previous run is also a failure, the badge counts 1 but the inbox shows 0.

Confidence Score: 2/5

  • Not safe to merge — the dismiss route is missing a company access check, enabling cross-tenant data modification in multi-tenant deployments.
  • Two logic bugs need fixing: the missing assertCompanyAccess on the dismiss endpoint (security concern) and the badge/inbox count inconsistency in getLatestFailedRunsByAgent. All other changes (migration, schema, shared types, CLI defaults, process adapter) are straightforward and correct.
  • server/src/routes/agents.ts (missing access check) and ui/src/pages/Inbox.tsx (badge/inbox logic mismatch)

Important Files Changed

Filename Overview
server/src/routes/agents.ts Adds POST /heartbeat-runs/:runId/dismiss; the new route is missing assertCompanyAccess, allowing any board user to dismiss runs from any company.
ui/src/pages/Inbox.tsx Adds dismiss button, new/all tabs, category filter, and touched-issues section; getLatestFailedRunsByAgent doesn't skip dismissed runs when building the per-agent map, creating a badge/inbox count mismatch; dismissRunMutation lacks an onError handler.
server/src/services/heartbeat.ts Adds dismissRun service method that sets dismissedAt = now(); straightforward implementation, though re-dismissing an already-dismissed run will update its timestamp unnecessarily.
server/src/services/sidebar-badges.ts Correctly adds isNull(heartbeatRuns.dismissedAt) to the WHERE clause before DISTINCT ON so dismissed runs don't count toward the inbox badge.
packages/db/src/schema/heartbeat_runs.ts Adds nullable dismissedAt timestamp column; migration and schema are consistent and correct.
packages/shared/src/types/heartbeat.ts Adds dismissedAt: Date
server/src/adapters/process/execute.ts Injects PAPERCLIP_API_KEY from authToken into the process env when available; straightforward and correct.
ui/src/api/heartbeats.ts Adds dismiss(runId) API call posting to the new /heartbeat-runs/:runId/dismiss endpoint; no issues.
cli/src/commands/configure.ts Adds disableSignUp: false to defaultConfig(); formatting-only changes otherwise.
packages/db/src/migrations/0026_awesome_gateway.sql Single ALTER TABLE statement to add the dismissed_at column; correct and safe as a nullable column addition.

Comments Outside Diff (2)

  1. ui/src/pages/Inbox.tsx, line 124-128 (link)

    Badge count / inbox display inconsistency after dismissing a run

    getLatestFailedRunsByAgent picks the absolute latest run per agent (regardless of dismissedAt), then the failedRuns memo later filters out any with dismissedAt set. The sidebar badge service, on the other hand, uses isNull(heartbeatRuns.dismissedAt) in its SQL WHERE clause before DISTINCT ON, so it returns the latest non-dismissed run per agent.

    Consider this scenario for a single agent:

    • run1 (latest, failed, dismissedAt set)
    • run2 (second, failed, dismissedAt null)

    BadgeisNull filter drops run1; run2 becomes the "latest" non-dismissed run → badge shows 1
    InboxgetLatestFailedRunsByAgent picks run1 as latest, failedRuns memo drops it because r.dismissedAt is non-null, run2 is never considered → inbox shows 0

    The badge and inbox are out of sync. The fix is to skip already-dismissed runs when building the per-agent map so the inbox applies the same "latest non-dismissed" selection logic that the badge SQL already uses:

    for (const run of sorted) {
      if (!latestByAgent.has(run.agentId) && !run.dismissedAt) {
        latestByAgent.set(run.agentId, run);
      }
    }
    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: ui/src/pages/Inbox.tsx
    Line: 124-128
    
    Comment:
    **Badge count / inbox display inconsistency after dismissing a run**
    
    `getLatestFailedRunsByAgent` picks the absolute latest run per agent (regardless of `dismissedAt`), then the `failedRuns` memo later filters out any with `dismissedAt` set. The sidebar badge service, on the other hand, uses `isNull(heartbeatRuns.dismissedAt)` in its SQL `WHERE` clause *before* `DISTINCT ON`, so it returns the latest **non-dismissed** run per agent.
    
    Consider this scenario for a single agent:
    - **run1** (latest, `failed`, `dismissedAt` set)  
    - **run2** (second, `failed`, `dismissedAt` null)
    
    **Badge**`isNull` filter drops run1; run2 becomes the "latest" non-dismissed run → badge shows `1`  
    **Inbox**`getLatestFailedRunsByAgent` picks run1 as latest, `failedRuns` memo drops it because `r.dismissedAt` is non-null, run2 is never considered → inbox shows `0`
    
    The badge and inbox are out of sync. The fix is to skip already-dismissed runs when building the per-agent map so the inbox applies the same "latest non-dismissed" selection logic that the badge SQL already uses:
    
    ```ts
    for (const run of sorted) {
      if (!latestByAgent.has(run.agentId) && !run.dismissedAt) {
        latestByAgent.set(run.agentId, run);
      }
    }
    ```
    
    How can I resolve this? If you propose a fix, please make it concise.
  2. ui/src/pages/Inbox.tsx, line 10213-10224 (link)

    No error feedback when dismiss API call fails

    dismissRunMutation has no onError handler. If the server returns an error (e.g. network failure, 404, 403), the card won't disappear (the local dismiss only runs in onSuccess), but there's also no message shown to the user. Consider adding an onError so users understand why the dismiss didn't take effect:

    onError: (err) => {
      setActionError(err instanceof Error ? err.message : "Failed to dismiss run");
    },
    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: ui/src/pages/Inbox.tsx
    Line: 10213-10224
    
    Comment:
    **No error feedback when dismiss API call fails**
    
    `dismissRunMutation` has no `onError` handler. If the server returns an error (e.g. network failure, 404, 403), the card won't disappear (the local dismiss only runs in `onSuccess`), but there's also no message shown to the user. Consider adding an `onError` so users understand why the dismiss didn't take effect:
    
    ```ts
    onError: (err) => {
      setActionError(err instanceof Error ? err.message : "Failed to dismiss run");
    },
    ```
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: server/src/routes/agents.ts
Line: 1574-1579

Comment:
**Missing company-level access check**

The dismiss endpoint only calls `assertBoard(req)`, which verifies the caller is an authenticated board user, but it never checks whether the caller actually belongs to the same company as the run being dismissed. In a multi-tenant deployment this allows any authenticated board user to dismiss heartbeat runs that belong to another organisation.

Compare to the events endpoint just below (line 1588), which correctly calls `assertCompanyAccess(req, run.companyId)` after fetching the run:

```ts
router.post("/heartbeat-runs/:runId/dismiss", async (req, res) => {
  assertBoard(req);
  const runId = req.params.runId as string;
  const run = await heartbeat.dismissRun(runId);
  if (!run) {
    res.status(404).json({ error: "Heartbeat run not found" });
    return;
  }
  assertCompanyAccess(req, run.companyId);
  res.json(run);
});
```

Note: the same issue exists on the pre-existing cancel route, but the new dismiss endpoint should not repeat the pattern.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: ui/src/pages/Inbox.tsx
Line: 124-128

Comment:
**Badge count / inbox display inconsistency after dismissing a run**

`getLatestFailedRunsByAgent` picks the absolute latest run per agent (regardless of `dismissedAt`), then the `failedRuns` memo later filters out any with `dismissedAt` set. The sidebar badge service, on the other hand, uses `isNull(heartbeatRuns.dismissedAt)` in its SQL `WHERE` clause *before* `DISTINCT ON`, so it returns the latest **non-dismissed** run per agent.

Consider this scenario for a single agent:
- **run1** (latest, `failed`, `dismissedAt` set)  
- **run2** (second, `failed`, `dismissedAt` null)

**Badge**`isNull` filter drops run1; run2 becomes the "latest" non-dismissed run → badge shows `1`  
**Inbox**`getLatestFailedRunsByAgent` picks run1 as latest, `failedRuns` memo drops it because `r.dismissedAt` is non-null, run2 is never considered → inbox shows `0`

The badge and inbox are out of sync. The fix is to skip already-dismissed runs when building the per-agent map so the inbox applies the same "latest non-dismissed" selection logic that the badge SQL already uses:

```ts
for (const run of sorted) {
  if (!latestByAgent.has(run.agentId) && !run.dismissedAt) {
    latestByAgent.set(run.agentId, run);
  }
}
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: ui/src/pages/Inbox.tsx
Line: 10213-10224

Comment:
**No error feedback when dismiss API call fails**

`dismissRunMutation` has no `onError` handler. If the server returns an error (e.g. network failure, 404, 403), the card won't disappear (the local dismiss only runs in `onSuccess`), but there's also no message shown to the user. Consider adding an `onError` so users understand why the dismiss didn't take effect:

```ts
onError: (err) => {
  setActionError(err instanceof Error ? err.message : "Failed to dismiss run");
},
```

How can I resolve this? If you propose a fix, please make it concise.

Last reviewed commit: e207ace

Comment on lines +1574 to +1579
router.post("/heartbeat-runs/:runId/dismiss", async (req, res) => {
assertBoard(req);
const runId = req.params.runId as string;
const run = await heartbeat.dismissRun(runId);
res.json(run);
});
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.

Missing company-level access check

The dismiss endpoint only calls assertBoard(req), which verifies the caller is an authenticated board user, but it never checks whether the caller actually belongs to the same company as the run being dismissed. In a multi-tenant deployment this allows any authenticated board user to dismiss heartbeat runs that belong to another organisation.

Compare to the events endpoint just below (line 1588), which correctly calls assertCompanyAccess(req, run.companyId) after fetching the run:

router.post("/heartbeat-runs/:runId/dismiss", async (req, res) => {
  assertBoard(req);
  const runId = req.params.runId as string;
  const run = await heartbeat.dismissRun(runId);
  if (!run) {
    res.status(404).json({ error: "Heartbeat run not found" });
    return;
  }
  assertCompanyAccess(req, run.companyId);
  res.json(run);
});

Note: the same issue exists on the pre-existing cancel route, but the new dismiss endpoint should not repeat the pattern.

Prompt To Fix With AI
This is a comment left during a code review.
Path: server/src/routes/agents.ts
Line: 1574-1579

Comment:
**Missing company-level access check**

The dismiss endpoint only calls `assertBoard(req)`, which verifies the caller is an authenticated board user, but it never checks whether the caller actually belongs to the same company as the run being dismissed. In a multi-tenant deployment this allows any authenticated board user to dismiss heartbeat runs that belong to another organisation.

Compare to the events endpoint just below (line 1588), which correctly calls `assertCompanyAccess(req, run.companyId)` after fetching the run:

```ts
router.post("/heartbeat-runs/:runId/dismiss", async (req, res) => {
  assertBoard(req);
  const runId = req.params.runId as string;
  const run = await heartbeat.dismissRun(runId);
  if (!run) {
    res.status(404).json({ error: "Heartbeat run not found" });
    return;
  }
  assertCompanyAccess(req, run.companyId);
  res.json(run);
});
```

Note: the same issue exists on the pre-existing cancel route, but the new dismiss endpoint should not repeat the pattern.

How can I resolve this? If you propose a fix, please make it concise.

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.

1 participant