Repository navigation
chore(deps): enforce seven-day third-party release-age policy - #819
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 gate: all four changed paths (.agents/skills/custom-codereview-guide.md, AGENTS.md, .github/dependabot.yml, pyproject.toml) are repository-owned configuration and review-policy content, with no SDK/automation/Canvas contract involved. In scope.
Re-established state: head is still 481398ccb340fab4d9e258b8f7b7f53077ba23df, PR is open, labels type: chore, no reviews and no comments on the head. Required checks (test, sync-extensions, validate-claude-code) pass; Validate PR description fails on this head.
What I verified on the head
- Config parses and behaves as described:
dependabot.ymlloads withcooldown.default-days: 7on the npm and pip entries only;pyproject.tomlloads withtool.uv.exclude-newer = "7 days". - The relative duration is valid: uv 0.12.15 accepts the "friendly duration" form and records
exclude-newer-span = "P7D"in the resolved lock.uv lockin the workspace produced no tracked change (uv.lockis gitignored). - Documentation claims check out against authoritative sources: npm's changelog shows
min-release-ageadded in 11.10.0 (so "requires npm >= 11.10" is right, and the local npm 11.21 accepts--min-release-age), and GitHub's Dependabot options reference confirmscooldownis version-updates-only and thatdefault-daysis supported for NPM and Pip. - Repository checks:
python scripts/sync_extensions.py --checkpasses with the pre-existing non-blocking coverage warning forissue-duplicate-checker, and the focused tests that touch this guide (test_sync_extensions.py,test_workflow_sync.py,test_pr_review_review_context.py,test_github_triage_delivery.py) pass: 80 passed. I did not run the full suite.
Findings
- Material: the rewrite silently drops the temporary-git-pin merge blocker. See inline comment.
- Non-blocking: the new
exclude-newerguard has no first-party exemption, unlike the review policy it is meant to back up. See inline comment. - Non-blocking, no inline location: the
github-actionsDependabot entry is left without a cooldown. GitHub's docs list GitHub Actions among the ecosystems supportingcooldown.default-days, so action updates can still be pulled the day they are released. The new policy text scopes itself to npm/PyPI, so this is consistent with the written rule, but the PR title claims a general "third-party release-age policy" and this ecosystem is the one left unguarded. - Non-blocking:
Validate PR descriptionfails on this head because## Issue NumberreadsN/Aand the checker requires a linked issue. That context is not in the ruleset's required set, so it does not block merge, but the check stays red until an issue is linked.
Verdict on the configuration and documentation itself: the npm/pip cooldowns, the uv cutoff, and the evidence/exception rules are accurate and consistent with the sources cited. Only finding 1 changes a merge decision, and it is a small correction.
🔄 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 gate: the four changed paths are repository-owned dependency configuration (pyproject.toml, .github/dependabot.yml) and review/contributor policy docs (AGENTS.md, .agents/skills/custom-codereview-guide.md). In scope, no product/architecture decision needed.
Re-established state (fresh pass, head moved): head is 7b080ab6e8b3e1fbe3080f423e7c287263ede7b7, PR open, labels type: chore. Every current-head check is green, including test, sync-extensions, validate-claude-code, and Validate PR description (now linked to issue #820, which carries ready-for-dev and priority:low). My earlier review was on 481398c; the author's follow-up pushed a new head, so this is a new review of the new code.
Both prior findings are resolved on this head
- The temporary-git-pin/unreleased-API merge blocker is restored verbatim (guide lines 120-122), matching base wording, together with the package-manager/runtime installation instruction.
- The
exclude-newerguard now carries an exemption:exclude-newer-package = { openhands-sdk = false }(pyproject.toml:39). Verified thatfalsedisables the cutoff for that name:uv lockresolvesopenhands-sdk 1.57.0while third-party packages stay filtered (e.g.litellm 1.105.1, not the 1.106.2 uploaded 2026-10-21). - The declared floor is well chosen:
required-version = ">=0.9.25"is exactly the release that added per-packageexclude-newerdisabling (astral-sh/uv#16854); relative durations landed earlier in 0.9.17, so 0.9.25 covers both.uv lock --checkexits 0 with 145 packages, and the enforcement is real: an impossiblerequired-versionmakes bothuv lockanduv syncexit with an error.python scripts/sync_extensions.py --checkpasses with the pre-existing non-blocking coverage warning, and the focused tests pass (44 passed). I did not run the full suite.
Finding (blocking): the single-name exemption skews the first-party versions in the repository's own documented commands.
[tool.uv] is repo-wide, so it applies to every uv run in this repository, not just the test dependency group. AGENTS.md:139 documents uv run --with openhands-tools --with openhands-workspace --with openhands-agent-server python ... (and AGENTS.md:137 the plugin form) as the way to validate SDK imports. Run from the repository root on this head, only openhands-sdk is exempt, so the command now resolves a mixed set, while the same command in a directory without the cutoff resolves a coherent one:
- this head, repo root:
openhands-sdk 1.57.0,openhands-tools 1.54.0,openhands-workspace 1.54.0,openhands-agent-server 1.54.0 - same command, no cutoff: all four at
1.57.0
So the documented validation command no longer exercises a released, coherent set of first-party packages, which is the thing AGENTS.md:139 exists to check. The author's rationale scoped the exemption to "the direct first-party test dependency", but the setting's reach is the whole repository, so the effect is broader than the stated intent. This also diverges from the review policy's blanket first-party exemption (skills/code-review/SKILL.md). The smallest correction is to exempt the first-party family that these commands install (openhands-sdk, openhands-tools, openhands-workspace, openhands-agent-server), or, if only the test dependency should be exempt, say in AGENTS.md that the resolver guard is deliberately stricter than the review rule so the skew is intentional and documented.
Minor, non-blocking: AGENTS.md:242 says tool.uv.required-version "also guards direct uv sync and uv lock use outside Make." This repository has no Makefile (none exists anywhere in the tree, and none appears in history), so "outside Make" refers to nothing here and reads as copied from a repository that does have one. The substance of the sentence is correct; drop the Make clause.
The npm/pip Dependabot cooldowns, the uv cutoff, the restored blocker, and the evidence/exception rules are accurate and verified. Only the exemption-scope finding changes a merge decision, and it is a small correction.
🔄 CHANGES REQUESTED
Co-authored-by: openhands <openhands@all-hands.dev>
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).
Fresh review of head 6238c99 (state re-read immediately before publishing: PR open, head matches, issue #820 open with ready-for-dev/priority:low, no comments after the previous review, all head CI checks green).
Scope: In scope for this repository. The change covers only its own dependency-intake surfaces: npm/pip Dependabot cooldowns, the local uv resolver config, and the contributor/reviewer guidance in AGENTS.md and .agents/skills/custom-codereview-guide.md. The diff is limited to exactly the four files issue #820 scopes; the github-actions entry, catalogs, generated skills, and package versions remain untouched per the issue's non-goals.
Since the previous review (on 7963f4b): one commit, "docs(deps): simplify lockfile guidance", which shortens an AGENTS.md bullet by removing the note that repository resolver settings do not protect downstream pip install/uv tool install consumers and do not retroactively validate existing locks. The operative instruction is retained ("Use the committed lock for reproducible installs; review newly resolved artifacts before committing a lock update"), and documenting downstream propagation was an explicit non-goal of issue #820, so no required rule is weakened. The temporary-git-pin/unreleased-API merge blocker and the maintainer-approval exception rules remain in place.
Verification on this head:
.github/dependabot.ymladdscooldown.default-days: 7to exactly the npm and pip entries; thegithub-actionsentry is unchanged. Head CI is green (test,check,sync-extensions,sync-sdk-skill, PR description/title validation).pyproject.toml[tool.uv](required-version >=0.9.25,exclude-newer = "7 days", four named first-partyexclude-newer-packageentries) parses cleanly;uv lock --checkresolves 145 packages, the lock stays gitignored/untracked, and the working tree is clean. Previously measured on this configuration with real uv: youngest third-party artifact 7.2 days, first-partyopenhands-sdk 1.58.0(2.3 days) exempted by name only.- The 0.9.25 floor is justified against uv's release history (
exclude-newer-packagelanded in 0.9.25, relativeexclude-newerdurations in 0.9.17). - Focused tests re-run in the workspace: 126 catalog/sync tests pass;
sync_extensions.py --checkpasses with only the pre-existing non-blockingissue-duplicate-checkercoverage warning, which exists identically onmainand is not introduced by this PR.
Non-blocking observation (documented residual gap, unchanged by this commit): with a real uv 0.9.24, the parser warns on the exclude-newer-package table and prints required-version but still resolves the full dependency set without any cutoff. required-version cannot hard-fail the very parser it would need to reject, so within these four files the mitigation is the documented uv --version check, which AGENTS.md already states. This is a uv tool limitation rather than a defect in this change.
No material issues found.
✅ APPROVED
|
🧹 PR Artifact Cleanup Queued The |
HUMAN:
This PR proposes to fix the 7 day cooldown rule, which was applied in some cases and not others.
AGENT:
Audit of the seven-day third-party package intake policy.
Why
Weekly npm/pip bot scheduling does not guarantee minimum publication age. The review guide mentions seven days but contributor and resolver configuration do not enforce it.
Summary
Add npm/Python bot cooldowns and uv cutoff; make contributor and reviewer evidence/exception rules explicit.
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.
Issue Number
Fixes #820
How to Test
uv lock --offlinesucceeded without adding a tracked lock.scripts/sync_extensions.py --checkpassed with an existing nonblocking coverage warning for issue-duplicate-checker. No catalogs or generated skills changed.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
Ready for human review. 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: restored the existing temporary-git-pin/unreleased-API merge blocker and installation/runtime validation instruction. Exempted only the direct first-party SDK test dependency from the uv wait; its transitive third-party dependencies remain filtered. Declared uv >=0.9.25 in the resolver and AGENTS; uv 0.9.25
lock --checksucceeds (145 packages).sync_extensions.py --checkpasses with the existing nonblocking catalog coverage warning. GitHub Actions policy remains out of scope: this PR applies to npm/PyPI. Linked issue #820 records this scope.Current review follow-up (24 October)
Exempted the four named first-party packages used by the documented validation commands: SDK, tools, workspace, and agent-server. Their third-party dependencies retain the cutoff. Corrected the old-uv warning and removed the nonexistent Makefile reference.
Validation: real uv 0.9.25 resolves all four first-party packages at 1.58.0 in an isolated resolution fixture (365 packages), with no third-party artifact younger than seven days. Root
uv lock --checkpasses (145 packages); the generated lock remains untracked/ignored as before. Catalog/schema tests: 117 passed.sync_extensions.py --checkpasses with the pre-existing duplicate-checker coverage warning.