Skip to content

fix: close bug-hunter stale-state paths#137

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

fix: close bug-hunter stale-state paths #137

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: 22 additions & 0 deletions AGENTS.md
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -89,6 +89,28 @@ 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: 21 additions & 2 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,metadata")
.select("id,owner_id,title,file_name,source_path,import_batch_id,status,error_message,page_count,chunk_count,image_count,metadata")
.eq("id", id)
.eq("owner_id", user.id)
.maybeSingle();
Expand DownExpand Up@@ -170,6 +170,15 @@ 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@@ -194,7 +203,17 @@ export async function POST(request: Request, { params }: { params: Promise<{ id:
.select()
.single();

if (jobError) throw new Error(jobError.message);
if (jobError) {
const { error: rollbackError } = await supabase
.from("documents")
.update(rollbackDocumentPayload)

@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 Guard document rollback against newer reindex jobs

When enqueue fails, this rollback restores the old document snapshot using only the document id and owner. If another reindex request for the same document successfully inserts a job after this request's pre-insert document update but before this rollback runs, the rollback can put the document back into the old failed/count state while a fresh pending or processing job exists. Constrain the rollback to the exact temporary state (or re-check no newer job exists) so cleanup from one failed enqueue cannot clobber a newer enqueue.

Useful? React with 👍 / 👎.

.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);
}
return NextResponse.json({ job }, { status: 201 });
} catch (error) {
if (error instanceof AuthenticationError) return unauthorizedResponse();
Expand Down
23 changes: 21 additions & 2 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,metadata")
.select("id,owner_id,title,file_name,source_path,import_batch_id,status,error_message,page_count,chunk_count,image_count,metadata")
.eq("owner_id", user.id)
.in("id", documentIds);
if (documentError) throw new Error(documentError.message);
Expand DownExpand Up@@ -166,6 +166,15 @@ 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@@ -188,7 +197,17 @@ export async function POST(request: Request) {
})
.select("id")
.single();
if (jobError) throw new Error(jobError.message);
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);
}
results.push({ documentId: document.id, mode: parsed.mode, ok: true, jobId: job.id });
} catch (error) {
results.push({
Expand Down
26 changes: 24 additions & 2 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@@ -16,7 +16,9 @@ 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,locked_at,documents!inner(owner_id)")
.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)",
)
.eq("id", id)
.eq("documents.owner_id", user.id)
.maybeSingle();
Expand DownExpand Up@@ -74,7 +76,27 @@ 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) throw new Error(documentError.message);
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);

@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 Guard retry rollback against claimed jobs

When the document update fails after the retry reset, this rollback filters only by job id. The reset to pending is already committed, so a worker can claim the job in the gap and set it to fresh processing; this unconditional rollback would then overwrite that active lock/progress with the stale pre-retry state, making an in-flight worker look failed/queued and allowing duplicate retries. Guard the rollback on the row still being the unclaimed reset state, or move the job/document reset into one atomic operation.

Useful? React with 👍 / 👎.

if (rollbackError) {
throw new Error(`${documentError.message}; failed to roll back retried job state: ${rollbackError.message}`);
}
throw new Error(documentError.message);
}

return NextResponse.json({ job: data });
} catch (error) {
Expand Down
30 changes: 29 additions & 1 deletion src/app/api/upload/route.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -24,6 +24,8 @@ 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@@ -132,6 +134,8 @@ 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@@ -146,7 +150,19 @@ export async function POST(request: Request) {
.select()
.single();

if (jobError) throw new Error(jobError.message);
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);
}

await writeAuditLog(supabase, {
ownerId: user.id,
Expand All@@ -158,6 +174,18 @@ 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);

@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 Check cleanup delete results

Supabase query failures are returned in the resolved { error } value, not thrown, so this best-effort cleanup silently ignores a failed document delete. When an upload inserts the document row and then a later step fails, a normal PostgREST delete error would leave a queued document with no ingestion job and no orphan log; destructure the delete response and log/handle its error the same way the explicit job-insert rollback does.

Useful? React with 👍 / 👎.

} 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: 12 additions & 5 deletions src/components/forms/form-detail-page.tsx
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -69,8 +69,12 @@ function readSavedForms() {

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

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

function chipToneClass(tone: ServiceChipTone | null | undefined) {
Expand Down
14 changes: 14 additions & 0 deletions tests/forms-clipboard-fallback.test.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
import { readFileSync } from "node:fs";
import { describe, expect, it } from "vitest";

const source = readFileSync(new URL("../src/components/forms/form-detail-page.tsx", import.meta.url), "utf8");

describe("form detail clipboard fallback", () => {
it("falls back to selection-copy when clipboard.writeText rejects", () => {
expect(source).toContain("if (navigator.clipboard?.writeText)");
expect(source).toContain("await navigator.clipboard.writeText(value)");
expect(source).toContain("Fall through to the legacy selection path for restricted browser contexts.");
expect(source).toContain("document.execCommand?.(\"copy\")");
expect(source).toContain("finally {\n document.body.removeChild(textArea);\n }");
});
});
Loading
Loading