fix(api): soft-delete reports with requests

[mnkj0021]

Fixes #124

Summary

• cascade request soft-deletes to associated reports
• handle both single and bulk request deletion paths
• record the same deletion timestamp on the request and reports
• expose deletedAt in the report type/schema for persisted soft-delete state
• add endpoint coverage for the cascade

Validation

• corepack pnpm --filter shared build — passed on a fresh recheck
• corepack pnpm --filter api build — passed on a fresh recheck
• corepack pnpm exec vitest run apps/api/src/endpoints.test.ts — 161/161 passed
• git diff --check — passed
• branch is current with upstream main (0 commits behind)

GitHub's upstream CI workflows are still in action_required and need a Scope maintainer to approve the fork workflow run. The Microsoft CLA check passes.

[Cedric Vidal (cedricvidal)]

Thanks for picking up #124! The write side looks right: one shared timestamp, both delete paths covered, and the deletedAt: { $exists: false } guard keeps existing timestamps intact.

Blocking

1. The soft-delete never shows up for users

Nothing that reads reports filters on deletedAt, so reports for deleted runs are still returned everywhere:

• GET /api/v1/reports?projectId= (the top-level list, which is the main thing Deleting a run should also soft delete associated reports #124 is about)
• GET /api/v1/reports?requestId=, GET /reports/:id, GET /reports/:id/logs, GET /reports/:id/insights
• POST /reports/bulk-status and the bulk-summary $match
• GET /insights/:id/reports

Please add deletedAt: { $exists: false } to these reads, plus tests showing deleted reports are left out.

2. Add a CosmosDB index for the new deletedAt filter

Once report reads filter on deletedAt, reports needs a deletedAt index the way the other soft-deleted collections have one. Right now it only has { createdAt: -1 } and { requestId: 1 } (migration 002) plus { projectId: 1 } (025).

Please follow the existing soft-delete index pattern:

• Add a new migration, e.g. packages/db-migrations/src/migrations/031-add-reports-soft-delete-index.ts, modeled on 030-create-resource-indexes.ts:await ensureIndex(db.collection("reports"), { deletedAt: 1 }, {}, "reports", TAG);
  Use the ensureIndex helper from cosmos-index-helpers.ts, as migrations 025 and 030 do for projects and resources, with a down() that only logs instead of dropping the index.
• Register it in packages/db-migrations/src/required-migrations.ts, or the API readiness probe won't check that it has been applied.
• Use single-field indexes: Cosmos combines them for { projectId | requestId, deletedAt } filters, and the existing range index on createdAt already handles the createdAt sort. If a query ever sorts on a field that only has a unique index, use a two-field compound like { deletedAt: 1, id: 1 } from migration 024 (Error while adding a criterion #1192).
• Add the migration to the table in docs/architecture/db-migrations.md.
• Ideally, check it on the shared dev Cosmos instance (docs/shared-dev-infra.md) with getIndexes(), because Cosmos can drop an index without reporting an error.

Should fix

1. No test for the bulk path. Add a DELETE /api/v1/requests/bulk test that checks the reportCollection.updateMany cascade is called with only the ids that were actually deleted (existingIds), and not called when nothing matched.
2. New reports can still be created for deleted runs. POST /reports, /reports/trigger, /reports/bulk-create and /reports/bulk-trigger look up runs without filtering out deleted ones. This was already the case before the PR, but it goes hand in hand with the cascade.

Nit

In DELETE /requests/:id, if the report update fails after the run is deleted, retrying gets a 410 and the reports are never cascaded. The cascade is idempotent, so you could also run it in the 410 branch.

[Cedric Vidal (cedricvidal)]

Thanks for picking up #124! The write side looks right: one shared timestamp, both delete paths covered, and the deletedAt: { $exists: false } guard keeps existing timestamps intact.

Blocking

1. The soft-delete never shows up for users

Nothing that reads reports filters on deletedAt, so reports for deleted runs are still returned everywhere:

• GET /api/v1/reports?projectId= (the top-level list, which is the main thing #124 is about)
• GET /api/v1/reports?requestId=, GET /reports/:id, GET /reports/:id/logs, GET /reports/:id/insights
• POST /reports/bulk-status and the bulk-summary $match
• GET /insights/:id/reports

Please add deletedAt: { $exists: false } to these reads, plus tests showing deleted reports are left out.

2. Add a CosmosDB index for the new deletedAt filter

Once report reads filter on deletedAt, reports needs a deletedAt index the way the other soft-deleted collections have one. Right now it only has { createdAt: -1 } and { requestId: 1 } (migration 002) plus { projectId: 1 } (025).

Please follow the existing soft-delete index pattern:

• Add a new migration, e.g. packages/db-migrations/src/migrations/031-add-reports-soft-delete-index.ts, modeled on 030-create-resource-indexes.ts:

  await ensureIndex(db.collection("reports"), { deletedAt: 1 }, {}, "reports", TAG);

  Use the ensureIndex helper from cosmos-index-helpers.ts, as migrations 025 and 030 do for projects and resources, with a down() that only logs instead of dropping the index.
• Register it in packages/db-migrations/src/required-migrations.ts, or the API readiness probe won't check that it has been applied.
• Use single-field indexes: Cosmos combines them for { projectId | requestId, deletedAt } filters, and the existing range index on createdAt already handles the createdAt sort. If a query ever sorts on a field that only has a unique index, use a two-field compound like { deletedAt: 1, id: 1 } from migration 024 (#1192).
• Add the migration to the table in docs/architecture/db-migrations.md.
• Ideally, check it on the shared dev Cosmos instance (docs/shared-dev-infra.md) with getIndexes(), because Cosmos can drop an index without reporting an error.

Should fix

1. No test for the bulk path. Add a DELETE /api/v1/requests/bulk test that checks the reportCollection.updateMany cascade is called with only the ids that were actually deleted (existingIds), and not called when nothing matched.
2. New reports can still be created for deleted runs. POST /reports, /reports/trigger, /reports/bulk-create and /reports/bulk-trigger look up runs without filtering out deleted ones. This was already the case before the PR, but it goes hand in hand with the cascade.

Nit

In DELETE /requests/:id, if the report update fails after the run is deleted, retrying gets a 410 and the reports are never cascaded. The cascade is idempotent, so you could also run it in the 410 branch.

[mnkj0021]

Cedric Vidal (@cedricvidal) I addressed the review feedback across the branch and pushed the final follow-up in 20425ed.

• all report reads now exclude deleted reports, including top-level/request-scoped lists, point/log/insight reads, bulk status/summary, and insight→report references
• report creation/trigger paths now reject soft-deleted runs
• added migration 031 for the reports.deletedAt index, registered it for readiness, documented it, and corrected migration expectations
• bulk request deletion now cascades only the active existingIds and skips the report update when nothing matched
• retrying DELETE on an already-deleted request now idempotently completes the report cascade before returning 410
• fixed an adjacent API contract bug found while testing: BulkReportStatusInputSchema exposed reportIds while both the route implementation and portal client use requestIds
• regenerated the committed OpenAPI snapshot and website spec

Validation: 183/183 targeted API + migration tests pass; OpenAPI contract tests and snapshot pass; shared, db-migrations, and api TypeScript builds pass; git diff --check passes.

I did not run the optional shared-dev Cosmos getIndexes() check from this environment.

[mnkj0021]

Thanks for the detailed review. I addressed the blocking items and the should-fix/nit follow-ups on the current branch:

• all report read paths now exclude documents with deletedAt set, including project/request listings, report detail/logs/insights, bulk status/summary, and insight report references
• added migration 031 with a single-field reports.deletedAt index, registered it in required migrations, updated migration readiness tests, and documented it
• added bulk-delete cascade coverage using only existingIds and the no-match case
• report creation/trigger paths now exclude deleted runs
• retrying DELETE /requests/:id after the request is already soft-deleted now re-runs the idempotent report cascade before returning 410
• fixed a malformed migration-table newline introduced while applying the follow-up

Fresh validation on the current branch:

• shared build: passed
• telemetry build: passed
• api build: passed
• endpoints + migration tests: 183/183 passed
• git diff --check: passed

Latest cleanup commit: 4260969.

[Cedric Vidal (cedricvidal)]

Give me a bit of time to validate this one, anything touching the DB needs more care.

Have you been able to validate this works with CosmosDB or just MongoDB? There are some subtle differences that can bite.

[mnkj0021]

I have not validated this against a real CosmosDB instance yet; the completed validation so far is local/unit/API coverage rather than shared-dev Cosmos.

For the index path, migration 031 deliberately uses the existing Cosmos-safe ensureIndex() helper with a single-field { deletedAt: 1 } index, matching the pattern used by migrations 025/030. The new queries also stay within the existing single-field-index model you called out rather than introducing a new compound/sort assumption.

I do not have the repo shared-dev Cosmos credentials/environment available on this runner, so I do not want to claim Cosmos validation I have not performed. I will leave the branch unchanged while you validate it rather than churn a DB-sensitive PR.