Repository navigation
Conversation
Co-authored-by: openhands <openhands@all-hands.dev>
all-hands-bot
left a comment
There was a problem hiding this comment.
This review was posted by an AI agent (OpenHands).
Scope: this is a dependency-policy/config change owned by this repository (AGENTS.md, .agents/skills/custom-codereview-guide.md, .github/dependabot.yml, pyproject.toml, uv.lock), so it belongs here. I re-read the repo guidance, the current PR body/labels, the head checks, and the current discussion (no prior reviews or comments). Head is still 1b5ea96621b416f98e33ac71def305a5da88e006; CI on that head is green (unit-tests, backend, Validate PR description, check-pr-artifacts all success).
What I verified independently on the checked-out head:
- Parsed
pyproject.tomlanduv.lock:[tool.uv] exclude-newer = "7 days",exclude-newer-package = { openhands-sdk = false, openhands-workspace = false }, and the lock's matching[options]/[options.exclude-newer-package]blocks are present and consistent. - Compared parsed
[[package]]name/version records between base and head: 200 entries in both, byte-for-byte identical version set. The PR description's "no package changes" claim holds. - The two lock entries with sub-seven-day
upload-timeare exactly the two exempted first-party packages (openhands-sdk1.57.0,openhands-workspace1.57.0, uploaded 2026-10-21), consistent with the stated exemption rationale. No third-party package in the lock is under seven days. ghcheck runs on the head are all green, matching the reported pre-commit/uv lockresults.
One material compatibility defect is described inline. It is not a policy-content problem; the added guards are correct in intent and the reviewer/contributor guidance matches the repository's existing conventions. It is that the committed artifacts now require a newer uv than this repository declares, and that requirement is neither updated nor documented.
✅ CI on the head is green and the guard semantics are correct for current uv.
🔄 The repository's declared minimum uv (Makefile: REQUIRED_UV_VERSION := 0.8.13, plus the README uv sync --group dev path) can no longer read the committed uv.lock; the floor needs to be raised and documented.
✅ APPROVED would follow once the uv floor is aligned. As it stands: 🔄 CHANGES REQUESTED
all-hands-bot
left a comment
There was a problem hiding this comment.
This review was posted by an AI agent (OpenHands).
Scope
This belongs here: the repository owns its own Dependabot configuration, uv lock/settings, and the review guidance under .agents/skills/. No ownership or product-direction concern.
Findings
1. The committed uv.lock now requires uv >= 0.9.25, but the repository's own version gate still accepts 0.8.13 (material).
The new lock emits an [options] table with [options.exclude-newer-package] entries valued false. I reproduced the boundary in a scratch copy (no tracked files changed):
- Base
uv.lock(HEAD~1):uv lock --checksucceeds on uv 0.8.13, 0.9.0, 0.9.10, 0.9.20 and 0.9.24. - This PR's
uv.lock:uv lock --check/uv sync --frozenfail on uv 0.8.13 through 0.9.24 witherror: Failed to parse 'uv.lock' ... 11 | [options] ... data did not match any variant of untagged enum Helper, and succeed on 0.9.25, 0.10.0, 0.11.32 and 0.12.23. Isolated fixtures show the booleanfalsevalue is the binding constraint: a lock carrying onlyexclude-newer-spanstill parses on 0.9.24. - The friendly-duration form
exclude-newer = "7 days"inpyproject.tomlis additionally unsupported before uv 0.9.20 (settings-discovery TOML warning on 0.8.13-0.9.10).
Impact: Makefile still declares REQUIRED_UV_VERSION := 0.8.13, and make build runs check-uv-version followed by uv sync --dev. A contributor on uv 0.8.13-0.9.24 passes the gate ("uv version 0.9.24 meets requirements") and then fails on an unrelated-looking TOML parse error. CI is unaffected (astral-sh/setup-uv@v7 unpinned installs latest) and the image pins uv 0.12.23, so this only affects local development - but it breaks the contributor contract the Makefile gate exists to enforce.
Suggested: raise REQUIRED_UV_VERSION to 0.9.25 in Makefile (and note the minimum in README.md), or avoid the boolean form. This PR already updates the policy docs, so recording the floor next to them keeps config and gate consistent.
Checks
- Current head
1b5ea96621b416f98e33ac71def305a5da88e006matches the requested SHA; all checks on this head are green (backend, unit-tests, Validate PR description, pr-title, check-pr-artifacts, Build and Push Automation Image). - Intended semantics verified: with
exclude-newer-package = { <pkg> = false }under a 3650-day cutoff, uv resolves the newest release (idna 3.20); without it, uv falls back to a 2016-era release (idna 2.1). The first-party exemptions are therefore necessary for the pinnedopenhands-sdk==1.57.0/openhands-workspace==1.57.0(both uploaded 2026-10-21, about one day before this PR) and are correctly scoped per package rather than to their dependencies. - Confirmed the lock diff introduces no package, version or source changes - only the new
[options]block. - Dependabot
cooldown.default-days: 7is supported for thepipecosystem, and cooldown applies to version updates only (security updates bypass it), matching the documented policy.
🔄 CHANGES REQUESTED
Co-authored-by: openhands <openhands@all-hands.dev>
all-hands-bot
left a comment
There was a problem hiding this comment.
This review was posted by an AI agent (OpenHands).
Scope
Belongs here: repository-owned Dependabot config, uv settings/lock, Makefile, README, and review guidance. No ownership or product-direction concern.
Prior finding - fixed
The uv 0.9.25 floor is now correct and consistent (Makefile REQUIRED_UV_VERSION := 0.9.25, README prereq, tool.uv.required-version). Re-verified against the real repo files: uv 0.9.20/0.9.24 reject the lock ([options] ... did not match any variant of untagged enum Helper), while 0.9.25, 0.10.0, 0.11.32 and 0.12.23 accept it. uv lock --check and uv sync --frozen --group dev pass on 0.12.23 with no lock churn, and the Docker uv sync --locked ... --group production path passes. Lock package records are unchanged versus base, and the sort -V gate now rejects 0.8.13-0.9.24 and accepts 0.9.25+.
Finding - the added required-version guard is inert for this configuration, and the AGENTS.md claim about it is inaccurate (material)
AGENTS.md now states that tool.uv.required-version "also guards direct uv sync and uv lock use outside Make." That does not hold. exclude-newer-package = { ... = false } is unparseable below uv 0.9.25, and when a [tool.uv] setting fails to parse, uv discards the whole table and falls back to defaults, so required-version is never evaluated.
Demonstrated with the real pyproject.toml and no uv.lock, sweeping uv 0.8.13, 0.9.0, 0.9.10, 0.9.11, 0.9.13, 0.9.15, 0.9.17, 0.9.19, 0.9.20, 0.9.22 and 0.9.24: every one resolves 208 packages, exits 0, and writes a lock containing no [options] block and no exclude-newer - the seven-day cutoff is silently dropped - while emitting no Required uv version message. Only 0.9.25 writes the cutoff. A control isolates the cause: with required-version plus a friendly-duration exclude-newer only, uv 0.9.24 does print error: Required uv version >=0.9.25 does not match the running version 0.9.24.
So the guard is a no-op for this configuration on every version below the floor rather than a backstop. The fail-open path requires uv.lock to be absent; the committed lock makes old uv fail closed with a parse error, and that parse requirement is the protection that actually works. Practical exposure is therefore narrow, but for a PR whose purpose is enforcing a minimum release age, documenting a protection that does not exist is a false assurance worth correcting before merge.
Suggested: reword the AGENTS.md sentence so the lock's parse requirement and the Makefile gate are named as what enforces the floor, or state that required-version only applies when the rest of [tool.uv] parses.
Checks
- Head
a10f3a675998d108e622f52f16b47e633413a690matches; all checks green (backend, unit-tests, Build and Push Automation Image, Validate PR description, pr-title, check-pr-artifacts). pytest tests/: 1504 passed, 60 skipped, 1 failed, 559 errors. The single failure (test_storage.py::TestGetFileStoreFactory::test_default_path_works_without_env) and the 559 errors reproduce on the base commit and are environmental (no Docker daemon / testcontainers), not caused by this change.- Dependabot
cooldown.default-days: 7is valid for thepipecosystem and applies to version updates only, matching the documented policy.
🔄 CHANGES REQUESTED
| typeCheckingMode = "standard" | ||
|
|
||
| [tool.uv] | ||
| required-version = ">=0.9.25" |
There was a problem hiding this comment.
This guard does not fire for this configuration on the versions it targets, and the AGENTS.md sentence describing it as a backstop for non-Make uv sync/uv lock is inaccurate.
exclude-newer-package = { ... = false } is unparseable below uv 0.9.25; when a [tool.uv] setting fails to parse, uv drops the whole table and falls back to defaults, so required-version is never evaluated. With the real pyproject.toml and no uv.lock, uv 0.8.13 through 0.9.24 each resolve 208 packages, exit 0, write a lock with no [options] block and no exclude-newer (cutoff silently dropped), and print no Required uv version error. Only 0.9.25+ writes the cutoff.
Control: with required-version plus a friendly-duration exclude-newer only, uv 0.9.24 does emit error: Required uv version >=0.9.25 does not match the running version 0.9.24 - the boolean setting is what defeats the guard.
Please reword the AGENTS.md claim to name the lock parse requirement and the Makefile gate as the actual enforcement, or note that required-version only applies when the rest of [tool.uv] parses.
Audit of the seven-day third-party package intake policy.
Why
Weekly pip bot scheduling does not prevent updating to a freshly published version; the uv resolver and agent/reviewer instructions lack the minimum age.
Summary
Add Python bot cooldown and uv seven-day cutoff; synchronize lock options without package changes. Name only the two direct first-party packages as exemptions so the existing pinned release remains resolvable.
Require exact-version publication/upload evidence for direct and transitive third-party npm/PyPI changes; weekly scheduling is not minimum age.
Security updates can bypass Dependabot cooldown. Urgent exceptions require a named package/version/reason and explicit maintainer approval; agents cannot grant their own bypass.
Repository settings do not propagate to downstream PyPI installs or retroactively validate an existing lock.
Issue Number
N/A - focused dependency-policy audit follow-up.
How to Test
Changed-file pre-commit passed.
uv lock --no-build-package litellmresolved successfully; pinned uv 0.11.32 validated the final lock without format churn.Additional audit checks:
cooldown.default-days: 7.git diff --checkpassed. Compared parsed lock package records before/after: no package changes.--package-lock-only --ignore-scripts --no-audit --no-fund; uv usedlock --no-build --no-cache. No live third-party package was installed by these fixtures.Video/Screenshots
N/A - dependency configuration and contributor/reviewer instructions only; no UI or runtime feature change.
Type
Notes
Draft pending human review and any human-only template field. No merge requested.
Sources: Dependabot cooldown, uv exclude-newer, npm min-release-age. npm's manual
--min-release-age=7requires npm >=11.10; older versions do not enforce it, andnpm cireplays the lock rather than proving age.Review follow-up
Review follow-up: set
tool.uv.required-version = ">=0.9.25", raised the Makefile floor to 0.9.25, and documented the minimum in README/AGENTS. Real uv 0.9.24 rejects the boolean-exemption lock; uv 0.9.25lock --checksucceeds (200 packages).make check-uv-versionand changed-file pre-commit passed. Lock package records remain unchanged.