Fix catalog-item instance delete when placement run is already absent

[gciavarrini]

When a placement run is removed before its catalog-item instance is deleted, the delete call fails with HTTP 500 because the placement manager returns 404 and the service treats it as a hard error.
The DB record stays, and the parent catalog item cannot be deleted (HTTP 409).

Treat a placement 404 during instance delete as success (the run is already gone), removes the DB record, and unblocks parent deletion.

Fixes

FLPATH-4922

[qodo-code-review]

PR Summary by Qodo

Allow instance deletion when its placement run is already absent

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

AI Description

• Treat a missing placement run as already deleted during catalog-item instance cleanup.
• Remove the instance record so it no longer blocks parent catalog-item deletion.
• Add a regression test confirming deletion succeeds when placement returns 404.

Diagram

graph TD
  A["Delete request"] --> B["Instance service"] --> C["Placement delete"] --> D{"PM result"}
  D -->|"success or 404"| E["Instance store"] --> F["Record removed"]
  D -->|"other error"| G["Mapped error"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Make placement-client deletion idempotent

• ➕ Could apply the same missing-run behavior to both local and remote placement clients.
• ➖ Broadens the placement-client contract for every caller rather than limiting the exception to instance deletion.

Recommendation: Keep the service-level exception because it narrowly preserves existing handling for other failures. If split deployments must support this fix, also make the remote client's DeleteRun return a typed PlacementError: it currently returns a plain formatted error for HTTP 404, which this service check cannot recognize.

Files changed (2) +29 / -5

Bug fix (1) +14 / -5

catalog_item_instance.goContinue instance deletion when placement reports a missing run +14/-5

Continue instance deletion when placement reports a missing run

• Recognizes a 404 PlacementError from run deletion and proceeds to delete the instance record. Other placement errors retain their existing mapping and stop deletion.

internal/catalog/service/catalog_item_instance.go

Tests (1) +15 / -0

catalog_item_instance_test.goTest deletion when the placement run is absent +15/-0

Test deletion when the placement run is absent

• Adds a service regression test that returns a placement 404, expects deletion to succeed, and verifies the instance record is gone.

internal/catalog/service/catalog_item_instance_test.go

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

1. Remote deletes still fail for absent runs 🐞 Bug ≡ Correctness

Description

isPlacementNotFound accepts only a PlacementError, but the remote client's DeleteRun returns a
plain formatted error for HTTP 404. When a placement manager URL selects that client, instance
deletion returns a placement failure before removing the catalog record, leaving the parent deletion
blocked.

Code

internal/catalog/service/catalog_item_instance.go[273]

+		if isPlacementNotFound(err) {

Evidence

The configured placement manager URL selects the remote client. Its delete method returns an untyped
error for non-2xx responses, while the new helper recognizes only a typed PlacementError; the
catalog store deletion occurs only after that check.

internal/app/run.go[347-351]
internal/catalog/placement/client.go[173-181]
internal/catalog/service/catalog_item_instance.go[272-285]
internal/catalog/service/catalog_item_instance.go[325-330]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The remote placement client's `DeleteRun` turns HTTP 404 into an untyped error, so the new absent-run branch cannot recognize it.
## Fix Focus Areas
- internal/catalog/placement/client.go[173-181]
- internal/catalog/service/catalog_item_instance.go[272-282]
- internal/catalog/placement/client_test.go[187-201]
## Recommended Fix
Return a `PlacementError` containing the HTTP status and response body for non-2xx remote delete responses. Test that a remote 404 is recognized by catalog instance deletion and permits removal of the stored instance.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗

---

Context sources

⚠️ Tickets: not configured — ticket URL found in PR but could not be fetched — check ticket provider credentials

Review mode: ⚖️ Balanced: This is a localized behavioral change in deletion and error classification that affects cleanup and parent-deletion availability, warranting a careful single-pass review.

Tip of the day

💡 Did you know, you can show, collapse, or hide each part of a finding: code, evidence, and all

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[qodo-code-review]

1. Remote deletes still fail for absent runs 🐞 Bug ≡ Correctness

isPlacementNotFound accepts only a PlacementError, but the remote client's DeleteRun returns a
plain formatted error for HTTP 404. When a placement manager URL selects that client, instance
deletion returns a placement failure before removing the catalog record, leaving the parent deletion
blocked.

Agent Prompt

## Issue description
The remote placement client's `DeleteRun` turns HTTP 404 into an untyped error, so the new absent-run branch cannot recognize it.
## Fix Focus Areas
- internal/catalog/placement/client.go[173-181]
- internal/catalog/service/catalog_item_instance.go[272-282]
- internal/catalog/placement/client_test.go[187-201]
## Recommended Fix
Return a `PlacementError` containing the HTTP status and response body for non-2xx remote delete responses. Test that a remote 404 is recognized by catalog instance deletion and permits removal of the stored instance.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗