[BigSimmo]

User description

Summary

• Before: when an upload was retried after an earlier attempt with an unknown outcome, the retry found an identical file already at its intended storage path. It then stored a second recovery copy and admitted that one, leaving the first copy in private storage with nothing referencing it (Gitar follow-up from PR Reconcile admin ingestion, native PDF extraction and grounded Answer #3546).
• After: once this request's own admission is durable on the recovery copy, the collided object at the intended path is removed. If removal fails, it is queued on the existing storage_cleanup_jobs ledger. If both fail, the upload still succeeds and the path is logged for manual reconciliation.
• Why this is safe: the document id is fixed by the correlation id, so once this admission commits no other admission for that document can commit. Nothing else can reference the intended-path object. The release never runs before the admission is durable, and never touches the path the committed row uses (both pinned by tests).
• No database change. It uses the existing cleanup ledger table only.
• Records the owner's 16 Oct decision to leave a job stuck on its final attempt to the existing 6-hourly ingestion check and queue recovery, rather than a new database fix (docs/decisions/2026-10-16-stuck-final-attempt-uses-existing-recovery.md, docs/ingestion-state-machine.md C1a). Drops the two ledger inbox rows that this change and that decision close.

Areas touched: Source intake and indexing, Delivery and assurance, Knowledge and records

Verification

• Checks run:
  • npm run verify:cheap: Test Files 1943 passed (1943) and Tests 31531 passed | 2 skipped (31533), lint and typecheck clean.
  • npm run test:focused on the changed files: Tests 228 passed (228).
  • npm run check:decisions: 0 errors, 0 warnings.

Risk and rollout

• Risk: low. It only deletes an object under this document's own storage prefix, after its own admission committed, and only when the upload path recovered from a collision.
• Rollback: revert this PR. A leftover copy would then stay in storage as before.
• Provider or production effects: none beyond the normal app deploy on merge.

Clinical Governance Preflight

• Source, citation, fencing and server-only access checks retained. The admission and its stored source bytes are unchanged.
• No clinical content, answer generation, ranking or retrieval change.
• No patient data and no new provider call.
• General product legal/clinical classification and unrestricted release readiness established (unchanged by this PR).

Notes

• Draft until the owner marks it ready. Marking ready lets Gitar merge it once green.

🤖 Generated with Claude Code

https://claude.ai/code/session_01M6gRtN6r4vZDgDfzMtjs95

---

Generated by Claude Code

---

Summary by cubic

Removes the orphaned upload copy left behind when a retried upload collides with an identical file already at its intended storage path.

Previously, a retried upload with an unknown earlier outcome stored a second recovery copy and admitted it, leaving the first copy in private storage with nothing referencing it. Now, once this request's own admission is durable on the recovery copy, the collided object at the intended path is removed, or queued on the existing storage_cleanup_jobs ledger if removal fails. Each cleanup call has a 5-second deadline so a committed upload is not held open; if both fail, the failure reasons and path are logged and the upload still succeeds. This is safe because the document id is fixed by the correlation id, so no other admission for that document can commit.

• Records the owner's decision to leave a job stuck on its final attempt to the existing 6-hourly ingestion check and queue recovery, rather than adding a database fix. The check reads only the 25 oldest open jobs; queue recovery scans all open jobs but applies at most --limit actions (default 20) per run, via --apply, --yes, or an interactive confirm.
• No database change; only the existing cleanup ledger table is used.

^{Written for commit 8cdf07e. Summary will update on new commits.}

---

CodeAnt-AI Description

Clean up collided files after retried uploads succeed

What Changed

• After a retried upload is admitted using its recovery copy, the identical file left at the intended path is removed.
• If removal fails, the file is queued for cleanup; cleanup failures do not undo the successful upload.

Impact

✅ Fewer orphaned files after retried uploads
✅ Successful uploads despite cleanup failures

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

Summary by CodeRabbit

• Bug Fixes
  • When an upload is saved under a replacement storage path after a collision, the superseded file is now removed or queued for cleanup after the upload is committed.
  • Cleanup issues that cannot be recorded no longer cause an otherwise successful upload to fail.
• Documentation
  • Clarified how ingestion jobs with uncertain outcomes on their final attempt are identified and recovered.

[codeant-ai]

CodeAnt Nitpicks

2 code suggestions

1. lastIndexOf only proves an earlier assignment exists in the file; an assignment on another conditional path can pass while release runs before durability.

Logic error · tests/upload-duplicate-cleanup-ledger.test.ts:20-22

---

2. toContain only checks that this expression appears somewhere; both release calls can pass null or another path and this assertion still passes, leaving cleanup unwired.

Logic error · tests/upload-duplicate-cleanup-ledger.test.ts:24

[codeant-ai]

CodeAnt PR Risk: Medium Risk

• The PR needs attention before merging.
• Final-attempt ingestion jobs can remain stuck, and the autopilot may delay flagging them when they fall outside its 25-job sample.
• Before releasing the collided object on the recovery response path, verify the recovered document does not still reference that storage path.

Assessed commit: 16c0b22acc25

[qodo-code-review]

Qodo Fixer

No findings are available for this PR yet. Findings appear here once Qodo has reviewed the PR.

[gitar-bot]

Gitar has auto-approved this PR (configure)

[BigSimmo]

Re the CodeAnt risk summary on 16c0b22:

• "Verify the recovered document does not still reference the collided path" (recovery response path): checked, and it cannot. The replay lookup in admin_admit_trusted_document (supabase/migrations/20261013075944_administrator_txt_fenced_runtime_cutover.sql, near "admission_replay_conflict") returns a row only when the stored request_digest equals this request's digest. That digest covers the admission, including storage_path_sha256, which on this path is the recovery sibling. An earlier event committed on the intended path has a different digest, so it raises admission_replay_conflict. The route catches that and never reaches releaseSupersededUpload. Every replayed document therefore points at admissionStoragePath, which is passed as committedPath and is never removed.
• 25-job autopilot sample: fixed in 16c0b22 (docs only). The owner chose not to add a database fix for stuck final-attempt jobs.
• Two test-tightening nits on tests/upload-duplicate-cleanup-ledger.test.ts: optional. They stay as they are unless another code push is needed on this PR, in which case they will ride along.

---

Generated by Claude Code

[qodo-code-review]

Code Review by Qodo

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

No findings for this group

No findings for this group

No findings for this group

Resolved findings

1. Recovery can change jobs without --apply ✗ Dismissed

Description

The C1a guidance says queue recovery applies changes only with --apply, but the tool also applies
them after interactive confirmation or when given --yes alone. An operator running the tool
without --apply could therefore confirm a prompt while relying on the documented claim that the
run cannot modify jobs.

Code

docs/ingestion-state-machine.md[204]

+  `--limit` actions per run (default 20) and only with `--apply`, so with a larger backlog rerun

Relevance

●●● Strong

Accepted: documentation contradicts mutating behavior; team fixes unsafe operational guidance and
requires explicit apply gates.

PR-#2789
PR-#3522
PR-#2702

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new guidance says recovery acts only with --apply. The script instead sets shouldApply after
confirmation or when args.yes is true, and proceeds to the mutation path whenever shouldApply is
true.

docs/ingestion-state-machine.md[203-205]
scripts/recover-ingestion-queue.ts[181-198]

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 recovery guidance incorrectly says `--apply` is required to modify jobs; interactive confirmation and `--yes` also enable changes.

## Fix Focus Areas
- docs/ingestion-state-machine.md[203-205]
- docs/decisions/2026-10-16-stuck-final-attempt-uses-existing-recovery.md[6-6]

## Recommended Fix
Replace the `--apply`-only wording with the actual behavior: the tool applies changes with `--apply`, with `--yes`, or after interactive confirmation. Keep the action-limit guidance intact.

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

---

2. Failed cleanup loses its error details ✗ Dismissed

Description

releaseSupersededDocumentObject discards both the removal error and the ledger-insert error,
reducing a double failure to "unrecorded". When neither operation succeeds, the route logs the
path and owner but not why removal or durable queuing failed, leaving manual reconciliation without
the failure details.

Code

src/lib/trusted-document-admission.ts[R375-379]

+  try {
+    const { error } = await args.queueCleanup(supersededPath);
+    return error ? "unrecorded" : "queued";
+  } catch {
+    return "unrecorded";

Relevance

●●● Strong

Recent reviews favor preserving discarded errors and adding sanitized failure telemetry for
operational diagnosis.

PR-#3148
PR-#3560

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The helper catches or reduces both errors to an outcome string, and the route's sole double-failure
log contains no error message.

src/lib/trusted-document-admission.ts[369-380]
src/app/api/upload/route.ts[198-204]

Agent prompt

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

## Issue description
A double cleanup failure produces no ledger row and discards the errors needed to diagnose it.
## Fix Focus Areas
- src/lib/trusted-document-admission.ts[369-380]
- src/app/api/upload/route.ts[198-204]
## Recommended Fix
Return or otherwise propagate sanitized removal and ledger failure details to the route's manual-reconciliation log without changing the successful upload response.

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

---

3. Queue recovery can leave the job stuck ✗ Dismissed

Description

The C1a guidance says running queue recovery directly resets a final-attempt job, but
recover-ingestion-queue applies only the first 20 planned actions by default. When other recovery
actions consume that limit, the job remains untouched even though it was read, so an operator
following the stated recovery decision must rerun with a suitable limit or repeat the tool.

Code

docs/ingestion-state-machine.md[R203-204]

+  older jobs ahead of it clear. Queue recovery (T16) scans every open job, so running it directly
+  resets the job whether or not the autopilot has flagged it.

Relevance

●●● Strong

Recent reviews accept corrections when operational documentation could mislead operators about
recovery behavior.

PR-#3522

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new documentation promises that directly running recovery resets the job, whereas the tool
selects a limited prefix of actions and reports how many remain.

docs/ingestion-state-machine.md[197-204]
docs/decisions/2026-10-16-stuck-final-attempt-uses-existing-recovery.md[3-8]
scripts/recover-ingestion-queue.ts[99-117]
scripts/recover-ingestion-queue.ts[180-196]

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 documented final-attempt recovery promises a reset that one invocation of the existing tool does not guarantee.
## Fix Focus Areas
- docs/ingestion-state-machine.md[200-204]
- docs/decisions/2026-10-16-stuck-final-attempt-uses-existing-recovery.md[6-6]
## Recommended Fix
Clarify that the tool requires applying its plan, defaults to 20 actions, and may require a larger limit or repeated runs to reach this job; distinguish retry from superseding an already-indexed job.

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

---

View resolved (1)

4. Successful uploads can wait a minute ✗ Dismissed

Description

releaseSupersededUpload awaits object removal and, if that fails, awaits a cleanup-ledger insert
using an admin client with a 30-second timeout for each operation. When both operations time out,
the route can spend roughly another minute before responding to an upload whose admission has
already committed.

Code

src/app/api/upload/route.ts[R628-631]

+    await releaseSupersededUpload(adminSupabase, {
+      ownerId: uploadOwnerId,
+      documentId,
+      supersededPath: supersededStoragePath,

Relevance

●● Moderate

Timeout and stalled-request findings are accepted, but changing awaited cleanup is an architectural
latency tradeoff.

PR-#2967
PR-#3418

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The helper runs removal before ledger insertion, the route awaits it before returning, and the
default admin client applies a 30-second timeout to each request.

src/lib/trusted-document-admission.ts[369-380]
src/app/api/upload/route.ts[628-635]
src/lib/supabase/admin.ts[11-17]
src/lib/supabase/admin.ts[36-44]

Agent prompt

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

## Issue description
Sequential cleanup requests can delay the response for an already-committed upload by roughly 60 seconds.
## Fix Focus Areas
- src/app/api/upload/route.ts[176-205]
- src/lib/trusted-document-admission.ts[369-380]
## Recommended Fix
Give post-admission cleanup a small total time budget while preserving durable ledger fallback and the existing behavior that cleanup failure does not fail admission.

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

---

Context sources

✅ Compliance rules (platform): 9 rules

Review mode: Auto: ⏭️ Skipped: Documentation-only clarification with no behavioral or runtime changes.

Tip of the day

💡 Did you know, you can pick a display mode instead of tweaking every Display option by hand

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous reviews

Review updated until commit 8cdf07e

Results up to commit 16c0b22 ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

No findings for this group

No findings for this group

No findings for this group

Resolved findings

1. Queue recovery can leave the job stuck ✗ Dismissed

Description

The C1a guidance says running queue recovery directly resets a final-attempt job, but
recover-ingestion-queue applies only the first 20 planned actions by default. When other recovery
actions consume that limit, the job remains untouched even though it was read, so an operator
following the stated recovery decision must rerun with a suitable limit or repeat the tool.

Code

docs/ingestion-state-machine.md[R203-204]

+  older jobs ahead of it clear. Queue recovery (T16) scans every open job, so running it directly
+  resets the job whether or not the autopilot has flagged it.

Relevance

●●● Strong

Recent reviews accept corrections when operational documentation could mislead operators about
recovery behavior.

PR-#3522

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new documentation promises that directly running recovery resets the job, whereas the tool
selects a limited prefix of actions and reports how many remain.

docs/ingestion-state-machine.md[197-204]
docs/decisions/2026-10-16-stuck-final-attempt-uses-existing-recovery.md[3-8]
scripts/recover-ingestion-queue.ts[99-117]
scripts/recover-ingestion-queue.ts[180-196]

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 documented final-attempt recovery promises a reset that one invocation of the existing tool does not guarantee.
## Fix Focus Areas
- docs/ingestion-state-machine.md[200-204]
- docs/decisions/2026-10-16-stuck-final-attempt-uses-existing-recovery.md[6-6]
## Recommended Fix
Clarify that the tool requires applying its plan, defaults to 20 actions, and may require a larger limit or repeated runs to reach this job; distinguish retry from superseding an already-indexed job.

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

---

2. Failed cleanup loses its error details ✗ Dismissed

Description

releaseSupersededDocumentObject discards both the removal error and the ledger-insert error,
reducing a double failure to "unrecorded". When neither operation succeeds, the route logs the
path and owner but not why removal or durable queuing failed, leaving manual reconciliation without
the failure details.

Code

src/lib/trusted-document-admission.ts[R375-379]

+  try {
+    const { error } = await args.queueCleanup(supersededPath);
+    return error ? "unrecorded" : "queued";
+  } catch {
+    return "unrecorded";

Relevance

●●● Strong

Recent reviews favor preserving discarded errors and adding sanitized failure telemetry for
operational diagnosis.

PR-#3148
PR-#3560

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The helper catches or reduces both errors to an outcome string, and the route's sole double-failure
log contains no error message.

src/lib/trusted-document-admission.ts[369-380]
src/app/api/upload/route.ts[198-204]

Agent prompt

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

## Issue description
A double cleanup failure produces no ledger row and discards the errors needed to diagnose it.
## Fix Focus Areas
- src/lib/trusted-document-admission.ts[369-380]
- src/app/api/upload/route.ts[198-204]
## Recommended Fix
Return or otherwise propagate sanitized removal and ledger failure details to the route's manual-reconciliation log without changing the successful upload response.

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

---

3. Successful uploads can wait a minute ✗ Dismissed

Description

releaseSupersededUpload awaits object removal and, if that fails, awaits a cleanup-ledger insert
using an admin client with a 30-second timeout for each operation. When both operations time out,
the route can spend roughly another minute before responding to an upload whose admission has
already committed.

Code

src/app/api/upload/route.ts[R628-631]

+    await releaseSupersededUpload(adminSupabase, {
+      ownerId: uploadOwnerId,
+      documentId,
+      supersededPath: supersededStoragePath,

Relevance

●● Moderate

Timeout and stalled-request findings are accepted, but changing awaited cleanup is an architectural
latency tradeoff.

PR-#2967
PR-#3418

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The helper runs removal before ledger insertion, the route awaits it before returning, and the
default admin client applies a 30-second timeout to each request.

src/lib/trusted-document-admission.ts[369-380]
src/app/api/upload/route.ts[628-635]
src/lib/supabase/admin.ts[11-17]
src/lib/supabase/admin.ts[36-44]

Agent prompt

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

## Issue description
Sequential cleanup requests can delay the response for an already-committed upload by roughly 60 seconds.
## Fix Focus Areas
- src/app/api/upload/route.ts[176-205]
- src/lib/trusted-document-admission.ts[369-380]
## Recommended Fix
Give post-admission cleanup a small total time budget while preserving durable ledger fallback and the existing behavior that cleanup failure does not fail admission.

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

---

Results up to commit 580aa95 ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

No findings for this group

No findings for this group

No findings for this group

Resolved findings

1. Recovery can change jobs without --apply ✗ Dismissed

Description

The C1a guidance says queue recovery applies changes only with --apply, but the tool also applies
them after interactive confirmation or when given --yes alone. An operator running the tool
without --apply could therefore confirm a prompt while relying on the documented claim that the
run cannot modify jobs.

Code

docs/ingestion-state-machine.md[204]

+  `--limit` actions per run (default 20) and only with `--apply`, so with a larger backlog rerun

Relevance

●●● Strong

Accepted: documentation contradicts mutating behavior; team fixes unsafe operational guidance and
requires explicit apply gates.

PR-#2789
PR-#3522
PR-#2702

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new guidance says recovery acts only with --apply. The script instead sets shouldApply after
confirmation or when args.yes is true, and proceeds to the mutation path whenever shouldApply is
true.

docs/ingestion-state-machine.md[203-205]
scripts/recover-ingestion-queue.ts[181-198]

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 recovery guidance incorrectly says `--apply` is required to modify jobs; interactive confirmation and `--yes` also enable changes.

## Fix Focus Areas
- docs/ingestion-state-machine.md[203-205]
- docs/decisions/2026-10-16-stuck-final-attempt-uses-existing-recovery.md[6-6]

## Recommended Fix
Replace the `--apply`-only wording with the actual behavior: the tool applies changes with `--apply`, with `--yes`, or after interactive confirmation. Keep the action-limit guidance intact.

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

---

[BigSimmo]

Addressed in 580aa95:

• Qodo 1, failed cleanup loses its error details: fixed. releaseSupersededDocumentObject now returns { outcome, failures }, with a short reason for the removal and the ledger step. The route adds failures to the manual-reconciliation log. Tests pin both reasons.
• Qodo 2, queue recovery can leave the job stuck: fixed in the docs. C1a and the decision record now say that recovery applies at most --limit actions per run (default 20), and only with --apply. With a larger backlog, rerun it or raise the limit.
• Qodo 3, successful uploads can wait a minute: fixed. Post-admission cleanup uses its own admin client with a 5 s deadline per call, so the worst case is about 10 s. A timed-out removal still falls back to the ledger, and a failure still never fails the upload.
• CodeAnt nit 2 (toContain only): carried. The source-shape test now checks that each release call passes supersededPath: supersededStoragePath and committedPath: admissionStoragePath.
• CodeAnt nit 1 (lastIndexOf ordering check): stays as is. Each call is in the same block as the durableAdmission = true; before it, with no catch in between. The helper's own tests cover the behaviour, so a stricter source parse would add brittleness for little gain.

Checks: focused tests 228/228 passed, typecheck and eslint clean, check:decisions 0 errors.

---

Generated by Claude Code

[gitar-bot]

Gitar has auto-approved this PR (configure)

[qodo-code-review]

Code review by qodo was updated up to the latest commit 580aa95

[gitar-bot]

Gitar has auto-approved this PR (configure)

[gitar-bot]

Code Review ✅ Approved

🔴 High risk · Deleting the collided storage object after admission is durable could remove bytes still needed if the path-reference assumptions are wrong.

Removes the orphaned upload copy left at the intended storage path after a retried upload is admitted from its recovery copy, with cleanup queued on the existing storage_cleanup_jobs ledger if removal fails. The change also records the decision to leave stuck final-attempt jobs to the existing 6-hourly ingestion check. No issues found.

Verification

Rules No rules evaluated

Functional validation Not enabled

Set up

Auto-approval Approved

_{For more verification details, comment gitar display:verbose.}

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing compact verification results.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

---

Important

Your trial ends in 5 days — upgrade now to keep code review, CI analysis, auto-apply, custom automations, and more.

_{Was this helpful? React with 👍 / 👎 | Gitar}

[qodo-code-review]

Code review by qodo was updated up to the latest commit 8cdf07e