Skip to content

fix: keep budget views readable when a scope has been deleted - #11058

Open
markkuhr wants to merge 2 commits into
paperclipai:masterfrom
markkuhr:fix/deleted-agent-cleanup
Open

fix: keep budget views readable when a scope has been deleted#11058
markkuhr wants to merge 2 commits into
paperclipai:masterfrom
markkuhr:fix/deleted-agent-cleanup

Conversation

@markkuhr

@markkuhr markkuhr commented Aug 7, 2026

Copy link
Copy Markdown

Thinking Path

  • Paperclip is the open source app people use to manage AI agents for work
  • Budgets cap spend per company, project, or agent, and the overview renders every policy at once
  • A policy stores scopeType and scopeId, but scopeId is a soft reference with no foreign key, so nothing cascades when the scope is deleted
  • resolveScopeRecord threw notFound for a missing scope
  • Because the overview resolves every scope in one pass, one deleted agent took out the whole budget and dashboard view instead of one row
  • This pull request makes read paths degrade the affected row and leaves write paths strict
  • The benefit is that the budget view survives a deleted scope, however that scope came to be deleted

Linked Issues or Issue Description

No existing issue. Describing it here per CONTRIBUTING.md.

What happened?
When a budget policy points at an agent or project that no longer exists, the budget overview and dashboard fail with notFound instead of rendering.

Expected behavior
A scope that no longer exists degrades to a placeholder row. The rest of the view renders.

Steps to reproduce

  1. Create an agent and a budget policy scoped to it.
  2. Remove the agent row.
  3. Load the budget overview for the company. It fails instead of rendering.

Paperclip version or commit
master, a388ea1.

Deployment mode
Reproduced on local_trusted. Not mode specific.

Installation method
From source.

What Changed

  • resolveScopeRecord takes a strict option. When false it returns a tombstone record for a missing agent or project instead of throwing.
  • Read paths that resolve many scopes use the non-strict mode. Mutating paths keep strict behaviour.
  • Added a regression test that deletes the agent a policy points at, through agentService.remove, and then reads the overview.
  • The company scope stays strict in both modes. Its policy and incident rows carry a real foreign key to companies, so unlike an agent or project it cannot outlive its scope. Documented in place so the asymmetry does not read as an oversight.

Verification

npx vitest run server/src/__tests__/budgets-service.test.ts — 8 pass. pnpm --filter @paperclipai/server typecheck is clean.

Reverting budgets.ts and keeping the test makes it fail, and the failure is at overview, not at the delete:

Error: Agent not found
  at notFound src/errors.ts:25:10
  at resolveScopeRecord src/services/budgets.ts:115:21
  at buildPolicySummary src/services/budgets.ts:318:19
  at Object.overview src/services/budgets.ts:632:24

The test deletes through agentService.remove rather than the table, and asserts the delete returned a row, so it also proves the dangling-scope state is reachable in production rather than only constructible in a test. Agent deletion clears its own dependent rows but never touches budget_policies, and budget_policies.scope_id carries no foreign key.

Risks

Low, and deliberately asymmetric. Only read paths degrade; write paths still reject a missing scope, so a policy cannot be created against a scope that does not exist. The tombstone is display only and carries paused: false, so it cannot cause a spurious pause.

Behavioral shift worth noting: the overview now renders rows for scopes that no longer exist, where it previously failed outright. That is the intent, but an operator can now see a policy whose target is gone. Cleaning up such policies is out of scope.

Scope note: this PR previously also changed agent deletion. That half is split out to #11062 after review feedback, because clearing an agent's cost events rewrites spend history and the fix for it needs cost_events.agent_id to stop being notNull, which is a schema migration.

Known adjacent bug, deliberately not fixed here: an agent that has accrued cost events, or is assigned to a routine, still cannot be deleted at all — cost_events.agent_id is notNull with a foreign key and routines.assignee_agent_id is a foreign key with no onDelete. That is #11062. It narrows which agents can reach the dangling-scope state; it does not remove it, since an agent with neither deletes cleanly today.

Model Used

Claude Opus 4.5 (claude-opus-4-5), extended thinking, 1M context, with tool use and code execution.

Checklist

  • I have included a thinking path that traces from project context to this change
  • I have specified the model used (with version and capability details)
  • I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
  • I have searched GitHub for duplicate or related PRs and linked them above
  • I have either (a) linked existing issues OR (b) described the issue in-PR following the relevant issue template
  • I have not referenced internal/instance-local Paperclip issues or links
  • My branch name describes the change and contains no internal ticket id
  • I have run tests locally and they pass
  • I have added or updated tests where applicable
  • I have updated relevant documentation to reflect my changes
  • I have considered and documented any risks above
  • All Paperclip CI gates are green
  • Greptile is 5/5 with no open P2s, recommendations, or follow-ups
  • I will address all Greptile and reviewer comments before requesting merge

@commitperclip

commitperclip Bot commented Aug 7, 2026

Copy link
Copy Markdown

✅ All checks passing — ready for Greptile review and maintainer approval.

— commitperclip

@greptile-apps

greptile-apps Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR keeps budget read paths available when an agent or project referenced by a policy has been deleted, while retaining strict scope validation for writes.

  • Adds non-strict scope resolution that returns display-only tombstones for missing agents and projects.
  • Applies tolerant resolution to policy summaries, incident payload construction, and incident listing.
  • Updates the regression test to delete the agent through the production service path before loading the overview.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
server/src/services/budgets.ts Adds tolerant missing-scope handling to budget read paths while preserving strict policy-upsert validation.
server/src/tests/budgets-service.test.ts Verifies through the real agent-deletion service path that a dangling agent-scoped policy no longer breaks the overview.

Reviews (4): Last reviewed commit: "test: delete the agent through the real ..." | Re-trigger Greptile

Comment thread server/src/services/agents.ts Outdated
Comment thread server/src/services/agents.ts Outdated
scopeId on a budget policy is a soft reference with no foreign key, so an agent or
project can be deleted while policies and incidents still point at it.

resolveScopeRecord threw notFound in that case. The overview resolves every scope in
one pass, so a single deleted agent took out the whole budget and dashboard view
rather than degrading the one affected row.

Scope resolution now takes a strict flag. Read paths that render many scopes pass
false and get a tombstone record for a missing scope. Mutating paths keep the strict
behaviour, so writing a policy against a scope that does not exist still fails.
@markkuhr
markkuhr force-pushed the fix/deleted-agent-cleanup branch from 6ced1d2 to cbadac5 Compare August 7, 2026 20:02
@markkuhr markkuhr changed the title fix: clean up references to a deleted agent fix: keep budget views readable when a scope has been deleted Aug 7, 2026
Comment thread server/src/__tests__/budgets-service.test.ts
…bstone test

Review flagged that the regression test removed the agent row directly and so
never proved the dangling-scope state is reachable in production. Deleting via
agentService.remove instead makes the test carry that proof: agent deletion
clears its own dependent rows but never touches budget_policies, and scopeId has
no foreign key, so the policy is left pointing at a row that is gone.

The premise behind the review comment was that deletion always rolls back on
routine and cost-event foreign keys. That holds only for an agent that has
accrued cost events or is assigned to a routine; an agent with neither deletes
cleanly today, which is exactly the case this test now exercises.

Also documents why the company scope stays strict in both modes: its policy and
incident rows carry a real foreign key to companies, so it cannot outlive its
scope the way an agent or project can.
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