fix: close bug-hunter stale-state paths - #137
Merged
Merged
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:7dcafb0726
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| next_run_at: job.next_run_at, | ||
| completed_at: job.completed_at, | ||
| }) | ||
| .eq("id", id); |
There was a problem hiding this comment.
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 (jobError) { | ||
| const { error: rollbackError } = await supabase | ||
| .from("documents") | ||
| .update(rollbackDocumentPayload) |
There was a problem hiding this comment.
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 👍 / 👎.
| } catch (error) { | ||
| if (insertedDocumentId && insertedDocumentOwnerId && supabase) { | ||
| try { | ||
| await supabase.from("documents").delete().eq("id", insertedDocumentId).eq("owner_id", insertedDocumentOwnerId); |
There was a problem hiding this comment.
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 👍 / 👎.
BigSimmo added a commit
that referenced
this pull request
Jul 2, 2026
BigSimmo added a commit
that referenced
this pull request
Jul 30, 2026
… open Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
BigSimmo pushed a commit
that referenced
this pull request
Jul 30, 2026
Codex found a real hole in #137's gate, and it is wider than the single file it named. `mockupPatterns` matched `-mockups.tsx` only, so eight singular `-mockup.tsx` components missed it, and three of the five specs the advisory project runs carry `@mockup` without "mockup" in the filename. The failure mode is silent and total. Editing `src/components/tools-page-mockups/task-directory-mockup.tsx` on its own gave `ui_changed=true, advisory_ui_changed=false`: the production projects `grepInvert` its `@mockup` tag, the advisory lane never starts, and `ui-tools-task-directory.spec.ts` — that component's only browser coverage — runs nowhere, on a green pull request. Matched now by mockup component directory as well as singular/plural filename, plus the three name-less advisory specs. A second list is how this happened, so the second list is now checked against the first. `assertMockupSpecParity` reads `mockupSpecPattern` out of playwright.config.ts, expands the alternation, and asserts every advisory spec matches `mockupPatterns`. It fails CLOSED on a lost anchor rather than quietly becoming a guard that checks nothing. Both directions proven by mutation: dropping the new spec pattern reports `tests/ui-tools.spec.ts` unrun, and renaming the constant in playwright.config.ts reports the missing anchor. Six self-test cases cover the singular filename, the mockup directory, both name-less specs, and the negative case that the directory rule does not swallow ordinary component paths. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JHLPEV4o1rzipPDqshCSHY
BigSimmo added a commit
that referenced
this pull request
Jul 30, 2026
* docs: file loose dated records and close the docs index gaps Organisation pass over the documentation surface and the orientation maps. No product code, schema, or retrieval/ranking surface is touched. Five dated point-in-time docs were sitting loose at the docs/ top level even though docs/README.md documents where they belong: - audit-handover-2026-07-14.md, audit-remediation-plan-2026-07-14.md and design-audit-2026-07-17.md move to docs/audit/ - rag-scalability-wip-remediation-2026-07-17.md (self-labelled "Superseded historical record") and rag-scalability-wip-review-handover-2026-07-15.md move to docs/archive/ Relative links inside the moved files are re-anchored to ../, and the two inbound references are updated in docs/operator-backlog.md and docs/archive/design-qa-2026-07-15.md. Contents are otherwise unchanged apart from Prettier's table reflow. The root codex-cloud-review/ directory moves to docs/prompts/codex-cloud-review/, where review prompts already live; .prettierignore and .cursorindexingignore are repointed so those verbatim inputs stay unformatted and unindexed. docs/README.md gains the seventeen docs it never listed — including search-chrome-behaviour.md, which AGENTS.md requires reading before search-chrome work — plus a subdirectory map for rag-behaviour/, prompts/, codex/, evidence/, audit/, redesign/, superpowers/ and archive/. CLAUDE.md and docs/codebase-index.md now document the top-level directories they omitted. Root data/ is the notable one: seven src/lib modules load it at runtime and it is easily confused with src/data/. scripts/check-docs-links.mjs gains a VERBATIM_DIRS skip for codex-cloud-review/. Relocating that directory under docs/ brought it into the default scan, where two of its internal references point at a path that never existed; the files are kept byte-for-byte as provided, so the reference cannot be corrected. This mirrors the existing archive/ and audit/ skip, and --all still sweeps them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(ledger): record the docs organisation review for PR #1436 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * issues: capture the root-dir coverage gap, the four unfileable dated docs, and the cross-worktree pre-commit hook Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * issues: keep the hook path out of the docs link checker Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(scripts-index): name every top-level script and correct the stale counts The index claimed ~135 files and ~166 npm entries against an actual 188 and 203, and 53 top-level scripts were unnamed — including ones AGENTS.md tells agents to run by name: branch-review-ledger.mjs (the only sanctioned way to read or write the review ledger), reconciliation-preflight.mjs, test-focused.mjs, check-hosted-migration-role.mjs and sync-open-pr-branches.mjs. Every one of the 168 top-level .mjs/.ts/.cjs scripts is now named, grouped by purpose in the doc's existing style rather than itemised per file, with new subsections for the review ledger/branch/skill tooling, live and staging verification, and browser and performance capture. The headline counts keep the exact '(N files)' / '(N entries)' shape and use the recursive scripts/ file count, because in-flight tooling in the primary checkout rewrites that sentence by regex; a note above the sentence records that contract so it is not reworded back into prose. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * issues: record the pre-commit fail-open mitigation and why #137 stays open Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: record final organization review * docs: format reconciled issue ledger * docs: correct historical CI readiness verdict * feat(gates): make docs:check-index cover tracked repo-root directories check-codebase-index-coverage.mjs exists to catch "a module that never gets added to the map, silently staling it", but it only looked inside src/ and at supabase/schema.sql. Nothing looked at repo-root directories, which is how root data/ came to be loaded at runtime by seven src/lib modules while appearing in neither CLAUDE.md nor docs/codebase-index.md — and how it stayed easy to confuse with the unrelated src/data/. The new root pass discovers directories from `git ls-files`, not the filesystem: the index documents the repository rather than one machine, so an untracked local scratch directory or node_modules can never fail the gate. Run red-then-green: the gate first reported .cursor, .design-sync and .vscode as unindexed — three real gaps — and passes once documented, covering 49 top-level entries where it previously covered 31. Also corrects two stale counts. docs/scripts-index.md said 188 files and 203 npm entries against an actual 191 and 206, having drifted within the hour as main merged new scripts. CLAUDE.md's hardcoded "~160 files backing 194 npm scripts" is removed rather than restated, because a hand-maintained count in a second place is what produced the drift; it now points at the scripts index. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * issues: correct #137 — the hook was committed all along, the MSYS ref:path check lied Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(ledger): record the docs:check-index root-coverage review for PR #1458 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: clarify untracked hook mitigation * docs: record reconciled PR #1458 review * docs: format archived issue record --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification