Feat: Unbounded Dependency Pins in requirements.txt (Closes #47)#53
Feat: Unbounded Dependency Pins in requirements.txt (Closes #47)#53bradjin8 wants to merge 6 commits into
requirements.txt (Closes #47)#53Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds bounded Pillow constraints and a pip-compile-generated lockfile, automates lock regeneration and Dependabot updates, updates CI to use the lockfile, updates README, and refactors the Cursor export pipeline to use shared services and a new IDE Markdown exporter. ChangesDeterministic dependency management via lock file
Export pipeline refactor
Sequence Diagram(s)sequenceDiagram
participant ExportAPI as api/export_api.py
participant WorkspaceService as services.workspace_listing / services.workspace_db
participant GlobalDB as global SQLite (cursorDiskKV)
participant MDExporter as utils.cursor_md_exporter.cursor_ide_chat_to_markdown
participant ZipBuilder as in-memory zip/manifest writer
ExportAPI->>WorkspaceService: collect workspace entries & composer→workspace map
WorkspaceService->>GlobalDB: open global DB & load bubble/layout/diff maps
ExportAPI->>MDExporter: call cursor_ide_chat_to_markdown for composer rows
MDExporter->>ExportAPI: return Markdown file bytes
ExportAPI->>ZipBuilder: add exported files and manifest entries
ZipBuilder->>ExportAPI: return zip bytes / persist export state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/update-lock.yml:
- Line 50: Replace the floating tag for the GitHub Action usage of
peter-evans/create-pull-request (currently "uses:
peter-evans/create-pull-request@v7") with a specific immutable commit SHA;
locate the "uses: peter-evans/create-pull-request@v7" line in the workflow, find
the latest verified commit SHA for that action (from the action's repository
tags or the marketplace), and update the string to "uses:
peter-evans/create-pull-request@<commit-sha>" so the workflow is pinned to a
specific commit instead of the v7 tag.
In `@requirements-lock.txt`:
- Line 16: The lockfile currently pins pillow==10.4.0 which contains multiple
high-severity vulnerabilities; update the upstream constraint that produced this
lock (the requirement spec "pillow>=10.3.0,<11") to allow Pillow 12.x (e.g.,
change to "pillow>=12.1.1" or "pillow>=12.2.0") after verifying that dependent
packages (notably fpdf2) are compatible with Pillow 12.2.0, then regenerate the
lock file (re-run your dependency lock/compile step) so the lock reflects
pillow==12.2.0 (or the chosen 12.x release) and ensure CI/tests pass.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 67859c4b-d312-4b72-b22c-53f21ba2fc9c
📒 Files selected for processing (6)
.github/dependabot.yml.github/workflows/tests.yml.github/workflows/update-lock.ymlpyproject.tomlrequirements-lock.txtrequirements.txt
|
README.md — Document lock-file workflow for contributors: CI uses requirements-lock.txt; how to regenerate (pip-compile command from the lock header); Dependabot + “Update dependency lock file” Actions. (Issue #47 asks for a documented update process; it lives only in YAML comments today.) .github/workflows/tests.yml (new step) — Add a lock freshness check, e.g. run pip-compile with the same flags as update-lock.yml and fail if requirements-lock.txt differs (git diff --exit-code). (Prevents requirements.txt / pyproject.toml edits without regenerating the lock while CI still passes.) .github/workflows/update-lock.yml:42-46 — Fix the “Restore header comment” step: HEADER is single-quoted, so \n is literal, not newline; printf '%s\n' "$HEADER" will write one malformed line when the workflow prepends the header after pip-compile --no-header. (The committed lock file is correct today; the first automated lock refresh will corrupt the header.) .github/dependabot.yml + update-lock.yml — Clarify the two-path maintenance model: Dependabot pip PRs may bump bounds in requirements.txt but do not refresh the lock; merging those should trigger (or be followed by) the lock workflow. (Reduces drift between bounds and pinned CI installs.) requirements.txt / pyproject.toml — Consider compiling from pyproject.toml (pip-compile pyproject.toml) or adding a CI check that [project.dependencies] matches requirements.txt. (Single source of truth; today sync is comment-only.) requirements-lock.txt — colorama is pinned as a transitive dep (Windows-oriented); harmless for reproducibility on Linux CI but matches the eval’s portability note. (No action required unless you want platform-specific lock files.) |
|
Here's details for the review comment.
Also fixed: Validation: |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
.github/workflows/update-lock.yml (2)
43-46: ⚡ Quick winAdd
--upgradeflag to the documented update command in the lock file header.The header's "Update:" line (which will appear in the generated
requirements-lock.txt) documents the pip-compile command for manual regeneration, but omits the--upgradeflag that this workflow actually uses on line 37. Developers copying the command from the header won't pick up newer package versions, contradicting the comment on lines 47-48 about picking up upstream releases.📝 Proposed fix
cat > /tmp/lock-header <<'EOF' # Pinned lock file — generated by pip-compile (pip-tools). # Install: pip install -r requirements-lock.txt - # Update: pip-compile requirements.txt --output-file requirements-lock.txt --no-header --annotation-style=line --allow-unsafe + # Update: pip-compile requirements.txt --output-file requirements-lock.txt --no-header --annotation-style=line --allow-unsafe --upgrade # Run periodically (e.g. via the "Update dependency lock file" CI workflow) to pick up # upstream patch / security releases within the bounded ranges in requirements.txt. EOF🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/update-lock.yml around lines 43 - 46, The "Update:" documentation line in the heredoc that writes /tmp/lock-header (the pip-compile usage string) is missing the --upgrade flag; edit that heredoc so the Update line reads "pip-compile requirements.txt --output-file requirements-lock.txt --no-header --annotation-style=line --allow-unsafe --upgrade" to match the workflow's actual behavior and ensure manual regeneration picks up upstream releases.
20-20: ⚡ Quick winConsider setting
persist-credentials: falsefor defense-in-depth security.By default,
actions/checkoutconfigures the GITHUB_TOKEN in the local git config. While this workflow doesn't execute untrusted repository code directly, pip-compile does resolve dependencies and can executesetup.pyduring that process. Settingpersist-credentials: falseprevents any such code from accessing the token via git commands.🔒 Proposed fix
- - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + with: + persist-credentials: false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/update-lock.yml at line 20, The checkout step using actions/checkout (the line with uses: actions/checkout@34e11...) should be updated to set persist-credentials: false to prevent GITHUB_TOKEN from being written to the repository git config; modify the checkout step configuration for that uses action to include the persist-credentials: false option so pip-compile or any dependency setup cannot access the token via git..github/workflows/tests.yml (1)
24-24: ⚡ Quick winConsider setting
persist-credentials: falsefor defense-in-depth security.By default,
actions/checkoutconfigures the GITHUB_TOKEN in the local git config. The lockfile job runs pip-compile, which can executesetup.pyduring dependency resolution. Settingpersist-credentials: falseprevents such code from accessing the token via git commands.🔒 Proposed fix
- - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + with: + persist-credentials: false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/tests.yml at line 24, The checkout step using actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 should be updated to include persist-credentials: false to avoid leaving GITHUB_TOKEN in the local git config; modify the checkout invocation (the actions/checkout usage) to add the persist-credentials: false option so subsequent commands like pip-compile cannot access the token via git.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/tests.yml:
- Around line 55-56: The "Install pip-tools" step currently installs pip-tools
without a version, causing non-reproducible lock verification; update that step
(the step named "Install pip-tools") to pin pip-tools to the same specific
version used by the update-lock.yml workflow (e.g., change the install command
to include the exact version specifier like pip-tools==<version>) so the
lockfile verification uses the same resolver version.
In @.github/workflows/update-lock.yml:
- Around line 27-28: Summary: The workflow step that runs "python -m pip install
pip-tools" installs an unpinned package which can yield non-reproducible lock
files. Fix: update the "Install pip-tools" run step to install a pinned
pip-tools version instead of the unversioned package; choose and hardcode the
exact pip-tools release used to generate the lock file (for example, the same
version used locally or in CI when creating the lock), so the command in that
step installs that exact version and ensures deterministic lock file generation.
---
Nitpick comments:
In @.github/workflows/tests.yml:
- Line 24: The checkout step using
actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 should be updated to
include persist-credentials: false to avoid leaving GITHUB_TOKEN in the local
git config; modify the checkout invocation (the actions/checkout usage) to add
the persist-credentials: false option so subsequent commands like pip-compile
cannot access the token via git.
In @.github/workflows/update-lock.yml:
- Around line 43-46: The "Update:" documentation line in the heredoc that writes
/tmp/lock-header (the pip-compile usage string) is missing the --upgrade flag;
edit that heredoc so the Update line reads "pip-compile requirements.txt
--output-file requirements-lock.txt --no-header --annotation-style=line
--allow-unsafe --upgrade" to match the workflow's actual behavior and ensure
manual regeneration picks up upstream releases.
- Line 20: The checkout step using actions/checkout (the line with uses:
actions/checkout@34e11...) should be updated to set persist-credentials: false
to prevent GITHUB_TOKEN from being written to the repository git config; modify
the checkout step configuration for that uses action to include the
persist-credentials: false option so pip-compile or any dependency setup cannot
access the token via git.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: db91b0fe-5302-413b-bc2b-8e61e768a3f4
📒 Files selected for processing (4)
.github/dependabot.yml.github/workflows/tests.yml.github/workflows/update-lock.ymlREADME.md
✅ Files skipped from review due to trivial changes (2)
- README.md
- .github/dependabot.yml
Extract _load_bubble_map/_load_project_layouts_map/_load_code_block_diff_map to services/workspace_db.py; add cursor_ide_chat_to_markdown to utils/cursor_md_exporter.py; remove private _slug() copies from api/export_api.py and utils/cursor_md_exporter.py.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/export_api.py (1)
210-214:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not return raw exceptions from this endpoint.
str(e)can include absolute paths, SQL details, and other local state. Log the exception server-side and return a generic 500 payload instead.Proposed fix
- except Exception as e: - print(f"Export error: {e}") - import traceback - traceback.print_exc() - return jsonify({"error": f"Export failed: {str(e)}"}), 500 + except Exception: + current_app.logger.exception("Export failed") + return jsonify({"error": "Export failed"}), 500🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api/export_api.py` around lines 210 - 214, The except block currently prints the exception and returns its stringified form via jsonify({"error": f"Export failed: {str(e)}"}), which can leak local paths/SQL; instead, log the full exception server-side (use your app logger or logging.getLogger(__name__) and include the traceback) and return a generic JSON error like jsonify({"error": "Export failed"}) with a 500 status; remove the print(...) and traceback.print_exc() calls and keep only the server-side logger call and the sanitized response in the except block.
🧹 Nitpick comments (1)
services/workspace_db.py (1)
38-43: ⚡ Quick winNarrow the per-row exception handling.
These shared loaders now feed both workspace listing and tabs. Catching
Exceptionhere will also hide real regressions and turn them into silent missing data; keep the row-level skip, but limit it to the JSON/type errors you actually expect.♻️ Suggested tightening
- except Exception: - pass + except (TypeError, ValueError): + continue ... - except Exception: - pass - except Exception: - pass + except (TypeError, ValueError): + continue + except (TypeError, ValueError): + continue ... - except Exception: - pass + except (TypeError, ValueError): + continueAlso applies to: 72-79, 101-109
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/workspace_db.py` around lines 38 - 43, The per-row try/except in services/workspace_db.py is too broad and hides real errors; tighten the exception handling around json.loads(row["value"]) and the isinstance check so only expected parsing/type errors are swallowed. Specifically, in the blocks that populate bubble_map (variables bid and b) and the similar blocks at the other noted ranges (lines approx. 72-79 and 101-109), replace "except Exception:" with a narrow clause such as "except (json.JSONDecodeError, TypeError, ValueError):" so malformed JSON or wrong types are skipped but other exceptions still surface.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@api/export_api.py`:
- Around line 151-165: The exporter currently builds searchable text only from
extracted bubble text (bubble_texts via extract_text_from_bubble) so exclusion
rules (is_excluded_by_rules) miss sensitive strings emitted as tool payloads or
synthetic code-edit diffs that cursor_ide_chat_to_markdown renders; update the
searchable content assembly used by build_searchable_text to also collect and
include tool payload text and any synthetic diff/code-edit text produced for
each bubble (the same pieces cursor_ide_chat_to_markdown emits) so
is_excluded_by_rules sees the exact content the exporter will output (use
bubble_map/headers loop to gather these extra strings alongside
extract_text_from_bubble before calling build_searchable_text).
In `@services/workspace_db.py`:
- Around line 97-107: The code currently builds diff entries from
row["key"].split(":") and appends them even when the third segment (diffId) is
missing, producing "diffId": None; update the guard in the block that computes
parts, cid and appends to diff_map so it only proceeds when len(parts) >= 3
(i.e., a valid third segment exists) — keep the existing variables (parts, cid,
d, diff_map) and only call diff_map.setdefault(cid, []).append(...) when the
third segment is present so tab.codeBlockDiffs never receives entries with
diffId == None.
In `@utils/cursor_md_exporter.py`:
- Around line 366-377: The frontmatter assembly in cursor_md_exporter.py builds
fm_lines with manually quoted title and workspace_name and an unquoted model,
which can produce invalid YAML for titles containing newlines, backslashes, or
quotes; update the block that appends title, workspace_name, and model to use
the same string-serialization routine as the CLI exporter (i.e., replace the
manual f-string quoting for title and workspace_name and the unquoted model
entry with the shared frontmatter serializer used elsewhere) so values are
properly escaped/quoted before being appended to fm_lines.
---
Outside diff comments:
In `@api/export_api.py`:
- Around line 210-214: The except block currently prints the exception and
returns its stringified form via jsonify({"error": f"Export failed: {str(e)}"}),
which can leak local paths/SQL; instead, log the full exception server-side (use
your app logger or logging.getLogger(__name__) and include the traceback) and
return a generic JSON error like jsonify({"error": "Export failed"}) with a 500
status; remove the print(...) and traceback.print_exc() calls and keep only the
server-side logger call and the sanitized response in the except block.
---
Nitpick comments:
In `@services/workspace_db.py`:
- Around line 38-43: The per-row try/except in services/workspace_db.py is too
broad and hides real errors; tighten the exception handling around
json.loads(row["value"]) and the isinstance check so only expected parsing/type
errors are swallowed. Specifically, in the blocks that populate bubble_map
(variables bid and b) and the similar blocks at the other noted ranges (lines
approx. 72-79 and 101-109), replace "except Exception:" with a narrow clause
such as "except (json.JSONDecodeError, TypeError, ValueError):" so malformed
JSON or wrong types are skipped but other exceptions still surface.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c1339897-1bfd-45a0-a440-8152f057033c
📒 Files selected for processing (7)
api/export_api.pyscripts/export.pyservices/workspace_db.pyservices/workspace_listing.pyservices/workspace_tabs.pyutils/cursor_md_exporter.pyutils/text_extract.py
This reverts commit 7926293.
2 participants
Summary
Closes #47 — Unbounded Dependency Pins in
requirements.txtChanges
Bounded specifiers
requirements.txt: added missing upper bound onpillow(>=10.3.0,<11).flaskandfpdf2already had upper bounds from Missing pyproject.toml / No Python Packaging #45; now all three runtime deps are fully bounded.pyproject.toml: samepillowupper bound applied to[project.dependencies]to keep both files in sync.Lock file (
requirements-lock.txt, new)pip-compile(pip-tools) from the boundedrequirements.txt.flask==3.1.3,fpdf2==2.8.7,pillow==10.4.0, plusblinker,click,colorama,defusedxml,fonttools,itsdangerous,jinja2,markupsafe,werkzeug.pip-compileinvocation to regenerate it.CI (
.github/workflows/tests.yml)unittestandtypecheckjobs now install fromrequirements-lock.txtfirst, then layer the dev-only dep (pytest>=8,<9/mypy>=1.10,<2) on top.pip install 'flask>=3.0' 'fpdf2>=2.7' ...invocations.Dependabot (
.github/dependabot.yml, new)github-actions(keeps the pinned SHA actions current) andpip(opens PRs for new versions within the bounded ranges inrequirements.txt).Scheduled lock-file refresh (
.github/workflows/update-lock.yml, new)pip-compile --upgradeevery Monday at 08:00 UTC.peter-evans/create-pull-requestwith the refreshedrequirements-lock.txtfor review before merging.Acceptance criteria checklist
requirements.txthave both lower and upper boundsrequirements-lock.txtgenerated and committedSummary by CodeRabbit