Skip to content

fix: claude-lane/task_1780281368494_s33opior9-20260602124820 #6

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
nehraa merged 1 commit into from
Jun 3, 2026
Merged

fix: claude-lane/task_1780281368494_s33opior9-20260602124820 #6

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
64 changes: 53 additions & 11 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 22 additions & 0 deletions src/service/coderag.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,33 @@ export class CodeRag {
return this.activeIndexPromise;
}

/**
* Waits for any in-flight index operation to settle so that subsequent
* operations observe a consistent state. Errors are intentionally swallowed
* here because the original caller of the index operation already receives
* the rejection through its returned promise.
*/
async waitForTermination(): Promise<void> {
if (this.activeIndexPromise) {
await this.activeIndexPromise.catch(() => {
/* original caller receives the error */
});
}
}

private async ensureLoadedState(): Promise<LoadedState> {
if (this.loadedState) {
return this.loadedState;
}

// Wait for any in-flight index operation to settle before checking disk state,
// so we never spawn a duplicate index run while another is still running.
await this.waitForTermination();
Comment on lines +84 to +105
Comment on lines +103 to +105

Comment on lines +103 to +106

@qodo-code-review qodo-code-review Bot Jun 3, 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.

Action required

2. Unbounded index wait 🐞 Bug ☼ Reliability

ensureLoadedState() now awaits waitForTermination(), which awaits activeIndexPromise without any
timeout; if an index operation never settles, lookups/queries that need loaded state can hang
indefinitely. Index operations include potentially long-running/remote vectorStore calls, while the
repo’s lock-wait path already has a timeout but is bypassed by this in-process wait.
Agent Prompt 
## Issue description
`CodeRag.ensureLoadedState()` now calls `await this.waitForTermination()`, and `waitForTermination()` awaits `activeIndexPromise` with no timeout. If the index promise never resolves/rejects (e.g., a stuck vector store call), all operations that require `ensureLoadedState()` can block indefinitely.

## Issue Context
- Indexing work includes `vectorStore.reset/upsert/deleteByNodeIds` which are async I/O and may stall.
- The codebase already has a bounded-wait pattern via `IndexLock.waitForRelease()` using `locking.timeoutMs`, but `waitForTermination()` does not apply any similar bound.

## Fix Focus Areas
- src/service/coderag.ts[68-123]
- src/store/index-lock.ts[56-75]
- src/indexer/indexer.ts[163-227]

## Suggested fix
- Add an optional `timeoutMs` parameter to `waitForTermination()` (defaulting to `this.config.locking.timeoutMs`).
- Implement the timeout with `Promise.race([activeIndexPromise.catch(...), timeoutPromise])`.
- On timeout, either:
  - throw an `IndexingError` explaining that an in-flight index did not terminate in time, or
  - log and fall back to the disk-lock wait path (e.g., `await this.indexer.waitForUnlockedState()`), so callers get a bounded failure mode consistent with other lock waits.

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

if (this.loadedState) {
return this.loadedState;
}
Comment on lines +84 to +109

@qodo-code-review qodo-code-review Bot Jun 3, 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.

Action required

1. No tests for waitfortermination() 📘 Rule violation ▣ Testability

Indexing coordination was changed by adding waitForTermination() and calling it from
ensureLoadedState(), but there is no test that exercises this new in-flight index waiting
behavior. This risks regressions where a query/lookup can race an in-progress index and reintroduce
duplicate index runs or inconsistent loaded state.
Agent Prompt 
## Issue description
`CodeRag.ensureLoadedState()` now waits for an in-flight index operation via the new `waitForTermination()` helper, but the test suite does not include coverage for this new coordination behavior.

## Issue Context
This change is part of indexing lifecycle/locking behavior (index creation/update coordination). Per compliance, modified indexing behavior must be covered by automated tests with assertions that would fail if the behavior is removed.

## Fix Focus Areas
- src/service/coderag.ts[84-123]
- src/test/coderag.test.ts[167-233]

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


const state = await this.indexer.loadState();
if (isStateLoaded(state.snapshot, state.documents)) {
return this.hydrateState(state.snapshot, state.documents);
Expand Down