Skip to content

revert: fix: close bug-hunter stale-state paths#142

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
BigSimmo merged 1 commit into from
Jul 2, 2026
Merged

revert: fix: close bug-hunter stale-state paths #142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 0 additions & 22 deletions AGENTS.md
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -89,28 +89,6 @@ After setup:

<!-- END:dependency-shortcut -->

<!-- BEGIN:bug-hunter-shortcut -->

## Bug-hunter shortcut

When the user types exactly `bug-hunter` as the entire task message, after trimming surrounding whitespace, treat it as a shortcut for targeted defect discovery.

Execution rules:

- Invoke the `bug-hunter` skill first.
- Prioritize reproducible defects over code style, naming, or formatting feedback.
- Trace realistic failure paths: invalid input, empty states, retries, race/concurrency issues, stale state/cache, network/auth failures, permissions, and boundary values.
- For each finding, include trigger, expected behavior, actual risk, and the smallest proof (or targeted test/check) that would catch it.
- If no high-confidence defect is found, explicitly state that and list the most likely residual risk area.

Scope and safety:

- Keep the hunt scoped to code touched by the user request unless the defect clearly crosses module boundaries.
- Do not make broad refactors while hunting; propose minimal fixes for confirmed issues.
- Run the smallest focused verification for each confirmed defect, then expand only if needed.

<!-- END:bug-hunter-shortcut -->

<!-- BEGIN:local-server-safety -->

# Local server safety
Expand Down
23 changes: 2 additions & 21 deletions src/app/api/documents/[id]/reindex/route.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -101,7 +101,7 @@ export async function POST(request: Request, { params }: { params: Promise<{ id:

const { data: document, error: documentError } = await supabase
.from("documents")
.select("id,owner_id,title,file_name,source_path,import_batch_id,status,error_message,page_count,chunk_count,image_count,metadata")
.select("id,owner_id,title,file_name,source_path,import_batch_id,status,metadata")
.eq("id", id)
.eq("owner_id", user.id)
.maybeSingle();
Expand DownExpand Up@@ -170,15 +170,6 @@ export async function POST(request: Request, { params }: { params: Promise<{ id:
}

const atomicReindex = isAtomicReindexCandidate(document);
const rollbackDocumentPayload = atomicReindex
? { error_message: document.error_message ?? null }
: {
status: document.status ?? null,
error_message: document.error_message ?? null,
page_count: document.page_count ?? 0,
chunk_count: document.chunk_count ?? 0,
image_count: document.image_count ?? 0,
};
const { error: updateError } = await supabase
.from("documents")
.update(
Expand All@@ -203,17 +194,7 @@ export async function POST(request: Request, { params }: { params: Promise<{ id:
.select()
.single();

if (jobError) {
const { error: rollbackError } = await supabase
.from("documents")
.update(rollbackDocumentPayload)
.eq("id", id)
.eq("owner_id", user.id);
if (rollbackError) {
throw new Error(`Failed to enqueue reindex job: ${jobError.message}; rollback failed: ${rollbackError.message}`);
}
throw new Error(jobError.message);
}
if (jobError) throw new Error(jobError.message);

@chatgpt-codex-connectorchatgpt-codex-connectorBotJul 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Restore document state if single reindex enqueue fails

For non-indexed full reindexes, the document is first changed to status: "queued" with page/chunk/image counts set to zero, but if the subsequent job insert fails this line returns an error with no job queued and no rollback. That leaves the document looking actively queued and emptied even though no worker can pick it up, so the previous indexed/failed state is lost until someone repairs the row manually.

Useful? React with 👍 / 👎.

return NextResponse.json({ job }, { status: 201 });
} catch (error) {
if (error instanceof AuthenticationError) return unauthorizedResponse();
Expand Down
23 changes: 2 additions & 21 deletions src/app/api/documents/bulk/reindex/route.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -91,7 +91,7 @@ export async function POST(request: Request) {
const documentIds = Array.from(new Set(parsed.documentIds));
const { data: documents, error: documentError } = await supabase
.from("documents")
.select("id,owner_id,title,file_name,source_path,import_batch_id,status,error_message,page_count,chunk_count,image_count,metadata")
.select("id,owner_id,title,file_name,source_path,import_batch_id,status,metadata")
.eq("owner_id", user.id)
.in("id", documentIds);
if (documentError) throw new Error(documentError.message);
Expand DownExpand Up@@ -166,15 +166,6 @@ export async function POST(request: Request) {
}

const atomicReindex = isAtomicReindexCandidate(document);
const rollbackDocumentPayload = atomicReindex
? { error_message: document.error_message ?? null }
: {
status: document.status ?? null,
error_message: document.error_message ?? null,
page_count: document.page_count ?? 0,
chunk_count: document.chunk_count ?? 0,
image_count: document.image_count ?? 0,
};
const { error: updateError } = await supabase
.from("documents")
.update(
Expand All@@ -197,17 +188,7 @@ export async function POST(request: Request) {
})
.select("id")
.single();
if (jobError) {
const { error: rollbackError } = await supabase
.from("documents")
.update(rollbackDocumentPayload)
.eq("id", document.id)
.eq("owner_id", user.id);
if (rollbackError) {
throw new Error(`Failed to enqueue bulk reindex job: ${jobError.message}; rollback failed: ${rollbackError.message}`);
}
throw new Error(jobError.message);
}
if (jobError) throw new Error(jobError.message);

@chatgpt-codex-connectorchatgpt-codex-connectorBotJul 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Restore each bulk reindex row when enqueue fails

In the bulk full/retry path, each document is mutated to queued and its counts are zeroed before inserting the ingestion job; if this insert fails, the per-document result reports failure but the document row is left in the queued/empty state with no pending job. This makes bulk reindex failures leave selected documents stuck as active work that no worker can process.

Useful? React with 👍 / 👎.

results.push({ documentId: document.id, mode: parsed.mode, ok: true, jobId: job.id });
} catch (error) {
results.push({
Expand Down
26 changes: 2 additions & 24 deletions src/app/api/ingestion/jobs/[id]/retry/route.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -23,9 +23,7 @@ export async function POST(request: Request, { params }: { params: Promise<{ id:

const { data: job, error: jobError } = await supabase
.from("ingestion_jobs")
.select(
"id,document_id,batch_id,status,stage,progress,error_message,attempt_count,max_attempts,locked_at,locked_by,next_run_at,completed_at,documents!inner(owner_id)",
)
.select("id,document_id,batch_id,status,locked_at,documents!inner(owner_id)")
.eq("id", id)
.eq("documents.owner_id", user.id)
.maybeSingle();
Expand DownExpand Up@@ -83,27 +81,7 @@ export async function POST(request: Request, { params }: { params: Promise<{ id:
.update({ status: "queued", error_message: null })
.eq("id", job.document_id)
.eq("owner_id", user.id);
if (documentError) {
const { error: rollbackError } = await supabase
.from("ingestion_jobs")
.update({
status: job.status,
stage: job.stage,
progress: job.progress,
error_message: job.error_message,
attempt_count: job.attempt_count,
max_attempts: job.max_attempts,
locked_at: job.locked_at,
locked_by: job.locked_by,
next_run_at: job.next_run_at,
completed_at: job.completed_at,
})
.eq("id", id);
if (rollbackError) {
throw new Error(`${documentError.message}; failed to roll back retried job state: ${rollbackError.message}`);
}
throw new Error(documentError.message);
}
if (documentError) throw new Error(documentError.message);

@chatgpt-codex-connectorchatgpt-codex-connectorBotJul 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Roll back the job reset if document requeue fails

If the guarded job update succeeds but the following document update returns an error, the API now reports failure while leaving the ingestion job reset to pending. claim_ingestion_jobs claims by job status, so a worker can retry the job even though the document status/error message was not synchronized and the user was told retry failed; restore the prior job snapshot when the document update cannot be committed.

Useful? React with 👍 / 👎.


return NextResponse.json({ job: data });
} catch (error) {
Expand Down
30 changes: 1 addition & 29 deletions src/app/api/upload/route.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -24,8 +24,6 @@ const uploadMetadataSchema = z
export async function POST(request: Request) {
let supabase: ReturnType<typeof createAdminClient> | null = null;
let uploadedPath: string | null = null;
let insertedDocumentId: string | null = null;
let insertedDocumentOwnerId: string | null = null;

try {
supabase = createAdminClient();
Expand DownExpand Up@@ -133,8 +131,6 @@ export async function POST(request: Request) {
.single();

if (documentError) throw new Error(documentError.message);
insertedDocumentId = documentId;
insertedDocumentOwnerId = user.id;

const { data: job, error: jobError } = await supabase
.from("ingestion_jobs")
Expand All@@ -149,19 +145,7 @@ export async function POST(request: Request) {
.select()
.single();

if (jobError) {
const { error: rollbackDocumentError } = await supabase
.from("documents")
.delete()
.eq("id", documentId)
.eq("owner_id", user.id);
if (rollbackDocumentError) {
throw new Error(`Failed to enqueue ingestion job: ${jobError.message}; rollback failed: ${rollbackDocumentError.message}`);
}
insertedDocumentId = null;
insertedDocumentOwnerId = null;
throw new Error(jobError.message);
}
if (jobError) throw new Error(jobError.message);

@chatgpt-codex-connectorchatgpt-codex-connectorBotJul 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Remove the inserted document when enqueue fails

When the storage upload and documents insert succeed but ingestion_jobs.insert() fails, this now throws without deleting the document row. The catch block still removes the uploaded storage object, so the user is left with a queued document whose storage_path points to a missing file; because duplicate detection runs by content_hash, retrying the same upload can also be treated as an existing duplicate and never queue a job.

Useful? React with 👍 / 👎.


await writeAuditLog(supabase, {
ownerId: user.id,
Expand All@@ -173,18 +157,6 @@ export async function POST(request: Request) {

return NextResponse.json({ document, job }, { status: 201 });
} catch (error) {
if (insertedDocumentId && insertedDocumentOwnerId && supabase) {
try {
await supabase.from("documents").delete().eq("id", insertedDocumentId).eq("owner_id", insertedDocumentOwnerId);
} catch (cleanupError) {
logger.error("Upload cleanup failed; document row may be orphaned", {
documentId: insertedDocumentId,
ownerId: insertedDocumentOwnerId,
message: cleanupError instanceof Error ? cleanupError.message : String(cleanupError),
});
}
}

if (uploadedPath && supabase) {
try {
await supabase.storage.from(env.SUPABASE_DOCUMENT_BUCKET).remove([uploadedPath]);
Expand Down
17 changes: 5 additions & 12 deletions src/components/forms/form-detail-page.tsx
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -69,12 +69,8 @@ function readSavedForms() {

async function copyText(value: string) {
if (navigator.clipboard?.writeText) {
try {
await navigator.clipboard.writeText(value);
return;
} catch {
// Fall through to the legacy selection path for restricted browser contexts.
}
await navigator.clipboard.writeText(value);

@chatgpt-codex-connectorchatgpt-codex-connectorBotJul 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Fall back when clipboard writes reject

In browsers or embedded contexts where navigator.clipboard.writeText exists but rejects because permission or context is restricted, this throws immediately and copyValue displays "Copy failed" without trying the legacy textarea selection path. Those contexts can still allow document.execCommand("copy"), so the copy action regresses for users who previously had the fallback.

Useful? React with 👍 / 👎.

return;
}

const textArea = document.createElement("textarea");
Expand All@@ -84,12 +80,9 @@ async function copyText(value: string) {
textArea.style.opacity = "0";
document.body.appendChild(textArea);
textArea.select();
try {
const copied = document.execCommand?.("copy");
if (copied === false) throw new Error("copy command rejected");
} finally {
document.body.removeChild(textArea);
}
const copied = document.execCommand?.("copy");
document.body.removeChild(textArea);
if (copied === false) throw new Error("copy command rejected");
}

function chipToneClass(tone: ServiceChipTone | null | undefined) {
Expand Down
14 changes: 0 additions & 14 deletions tests/forms-clipboard-fallback.test.ts
View file
Open in desktop

This file was deleted.

Loading
Loading