Repository navigation
fix: stale import path and resource leak cleanups - #102
Merged
ImKeTT merged 3 commits intoMar 19, 2026
Merged
Conversation
added 3 commits
March 19, 2026 14:39
The import `from arc.experiment_git import ExperimentGitManager` references the pre-rename module path. The actual class lives in `researchclaw.experiment.git_manager`. Any call passing `git_repo_dir` to ExperimentRunner raises ModuleNotFoundError. Made-with: Cursor
If any exception occurs between fitz.open() and doc.close(), the document object is never closed, leaking a file descriptor. During batch PDF processing in the literature collection stage this can exhaust the OS file descriptor limit. Replace manual open/close with a `with` statement so the document is always closed, even on error. Made-with: Cursor
mkstemp() returns a raw fd which is then passed to open(fd, "w"). If open() succeeds but the write fails, the with-block closes the fd, then the except handler calls os.close(fd) again. On systems where fd numbers are quickly reused, this can close an unrelated file descriptor. Simplify by closing the mkstemp fd immediately and reopening by path. The temp file already exists on disk so there is no TOCTOU race. Made-with: Cursor
This was referenced Mar 19, 2026
There was a problem hiding this comment.
Pull request overview
This PR consolidates three small fixes to address stale module references and ensure file descriptors are reliably cleaned up during PDF processing and checkpoint writing.
Changes:
- Fix stale
ExperimentGitManagerimport path inExperimentRunnerto preventModuleNotFoundError. - Prevent PyMuPDF document fd leaks by using a context manager around
fitz.open(). - Eliminate potential checkpoint fd double-close by closing the
mkstemp()fd immediately and reopening by temp path.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
researchclaw/experiment/runner.py |
Updates ExperimentGitManager import to the post-rename module path. |
researchclaw/web/pdf_extractor.py |
Wraps fitz.open() in a with block to guarantee document closure on exceptions. |
researchclaw/pipeline/runner.py |
Simplifies checkpoint temp-file handling to avoid ambiguous fd ownership and double-closing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Collaborator
|
thanks @sylearn this PR lgtm |
Jacky628
pushed a commit
to Jacky628/AutoResearchClaw
that referenced
this pull request
Aug 17, 2026
…esource-leak-cleanups fix: stale import path and resource leak cleanups
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Consolidates #96, #97, and #98 into a single PR as requested.
Three small, related fixes for stale references and resource management:
Stale import path (
experiment/runner.py):from arc.experiment_git import ExperimentGitManagerreferences a pre-rename module. Updated toresearchclaw.experiment.git_manager. Passinggit_repo_dirtoExperimentRunnercurrently raisesModuleNotFoundError.PyMuPDF fd leak (
web/pdf_extractor.py): If any exception occurs betweenfitz.open()anddoc.close(), the document is never closed. Replaced manual open/close with awithcontext manager to guarantee cleanup during batch PDF processing.Checkpoint fd double-close (
pipeline/runner.py):_write_checkpointpasses a rawmkstemp()fd toopen(fd, "w"). If the write fails, thewithblock closes the fd, then the except handler triesos.close(fd)again. Simplified by closing the fd immediately and reopening by path.Closes #96, #97, #98.
Test plan
researchclaw.experiment.git_manager.ExperimentGitManagerexists and definesis_git_repo()fitz.Documentsupports the context manager protocol