Skip to content

UN-1929 [FIX] Skip uv locking when lockfiles are already up to date - #2244

Open
Deepak-Kesavan wants to merge 1 commit into
mainfrom
UN-1929-uv-lock-automation
Open

UN-1929 [FIX] Skip uv locking when lockfiles are already up to date#2244
Deepak-Kesavan wants to merge 1 commit into
mainfrom
UN-1929-uv-lock-automation

Conversation

@Deepak-Kesavan

Copy link
Copy Markdown
Contributor

What

Makes the uv.lock automation a genuine no-op when the lockfiles in a PR are already correct, and stops the job failing when two runs overlap on the same branch.

Three changes:

  1. Gate on uv lock --check instead of "did pyproject.toml change vs origin/main?"
  2. uv syncuv lock
  3. Per-branch concurrency group on the workflow

Auto-commit behaviour is deliberately unchanged — the bot still commits lockfiles back to the PR branch exactly as before.

Why

1. The staleness gate asked the wrong question. has_dependency_changes() checked whether pyproject.toml (or a local path dependency's pyproject.toml) differed from origin/main. That is a proxy for the real question, "is uv.lock stale?", and it is wrong in exactly the case UN-1929 describes: a contributor (or their AI assistant) already ran uv lock and pushed correct lockfiles, but pyproject.toml still differs from main, so the gate trips and the job re-resolves everything anyway.

uv lock --check is the authoritative test — it is what uv exposes for this precise question.

2. uv sync did far more than needed. The workflow only ever commits uv.lock. uv sync additionally creates a virtualenv and installs every package in it — pure waste, and by far the largest failure surface in the job (network, sdist builds). uv lock resolves and writes the lockfile and nothing else.

3. The job's actual failure mode today is a push race. Reviewing the last 30 runs of this workflow, the failures are not re-lock failures at all:

error: failed to push some refs to 'https://github.com/Zipstack/unstract'
Error: Invalid status code: 1

There was no concurrency: block, so two pushes in quick succession start two overlapping runs, both of which try to auto-commit to the same branch; the loser fails the whole job and puts a red X on the PR. This accounted for 3 of the last 30 runs (e.g. branch UN-3987-fix-line-splitter-strategy-key, three overlapping runs within four minutes).

How

docker/scripts/uv-lock-gen/uv-lock.sh

  • Deleted get_local_dep_pyprojects() and has_dependency_changes() (~45 lines). uv lock --check subsumes both, including the transitive local-path-dependency walk they hand-rolled.
  • update_lockfile() now runs uv lock --check; if it passes, the directory is skipped entirely. Otherwise it runs uv lock.
  • The body runs in a subshell so the cd is scoped, replacing the manual cd/cd - dance.
  • Dropped git fetch origin main — nothing compares against origin/main any more. (This fetch was also responsible for the large * [new branch] dumps in the job logs.)

.github/workflows/uv-lock-automation.yaml

  • Added a workflow-level concurrency group keyed on the branch:
    concurrency:
      group: ${{ github.workflow }}-${{ github.head_ref || github.ref }}
      cancel-in-progress: true
    github.head_ref is empty on workflow_dispatch, hence the github.ref fallback.

Net: -46 lines, and the common case now does no dependency resolution at all.

Can this PR break any existing features. If yes, please list possible items. If no, please explain why.

No. This only touches the lockfile-automation CI job — no application code, no runtime path, no shipped artifact. Considered risks, each checked:

  • Could the new gate skip a directory that genuinely needs locking? Only if uv lock --check under-reports staleness. The one real concern was transitive local path dependencies, which the old code walked by hand. Verified explicitly (see Notes on Testing): editing unstract/core/pyproject.toml makes backend's uv lock --check exit non-zero. No regression in coverage.
  • Could uv lock produce a different lockfile than uv sync did? No — uv sync locks and then installs. The lockfile-producing step is identical; only the install is dropped.
  • Could cancel-in-progress drop a needed run? No. A run is only cancelled when a newer run in the same group starts, and that newer run operates on a strictly newer tree. If a subsequent push does not touch a pyproject.toml, the paths: filter means no new run starts, so the in-flight run is not cancelled and still completes.
  • Do genuine dependency errors still fail CI? Yes — verified with an unresolvable dependency; the script still exits 1.

Database Migrations

  • None.

Env Config

  • None.

Relevant Docs

Related Issues or PRs

Dependencies Versions

  • No dependency changes. uv stays pinned at 0.6.14; uv lock --check is available in that version (confirmed against the pinned version locally).

Notes on Testing

CI-only change with no UI or runtime surface, so this was verified by running the modified script directly against this repo with the same uv 0.6.14 that the workflow pins.

1. No-op path — the UN-1929 scenario. Clean tree, lockfiles already correct:

$ ./docker/scripts/uv-lock-gen/uv-lock.sh unstract/core unstract/flags backend
[unstract/core] uv.lock is already up to date, nothing to do
[unstract/flags] uv.lock is already up to date, nothing to do
[backend] uv.lock is already up to date, nothing to do

Exit 0, 0.69s for three directories, working tree untouched. Previously each of these would have run a full uv sync.

2. Stale path still works. Added tabulate to unstract/flags/pyproject.toml:

[unstract/flags] uv.lock is out of date, regenerating...
[unstract/flags] Resolved 7 packages in 4ms
[unstract/flags] Added tabulate v0.10.0

unstract/flags/uv.lock updated correctly (+46/-35).

3. Idempotency — re-running with the now-correct lock. This is the exact ticket scenario (lockfile already pushed):

[unstract/flags] uv.lock is already up to date, nothing to do

Second run touched nothing. This is what the ticket asked for.

4. Transitive local path dependencies — the main regression risk. Added a dependency to unstract/core/pyproject.toml, then checked backend (which consumes it via [tool.uv.sources] unstract-core = { path = "../unstract/core" }):

$ cd backend && uv lock --check
DETECTED (exit non-zero) -> transitive check works

So uv lock --check covers what get_local_dep_pyprojects() walked manually.

5. Failure propagation. With an unresolvable dependency pinned in unstract/flags/pyproject.toml, the script exits 1, so genuinely broken dependency changes still fail the job.

All test edits were reverted; backend/unstract/flags lockfiles confirmed current afterwards.

Note: this PR touches no pyproject.toml, so the paths: filter means the workflow will not self-trigger here. It can be exercised on this branch with
gh workflow run uv-lock-automation.yaml --ref UN-1929-uv-lock-automation.

Follow-up spotted while in here (not fixed in this PR)

The hardcoded directories list in uv-lock.sh has drifted. It covers 10 directories, but 14 have a uv.lock. These four are never maintained by the automation:

  • tool-sidecar
  • unstract/sdk1
  • unstract/tool-registry
  • unstract/tool-sandbox

Deliberately left out of scope — this PR is about doing less, and adding four directories is a behaviour change that deserves its own review. Worth noting that the uv lock --check gate makes adding them much cheaper than before, since up-to-date directories now cost effectively nothing. Happy to raise a follow-up ticket, or fold it in here if reviewers prefer.

Screenshots

N/A — CI-only change, no UI surface.

Checklist

I have read and understood the Contribution Guidelines.

The lock automation gated on "did pyproject.toml change vs origin/main?",
which is a proxy for the question it actually wanted to ask. A PR that
already carries correct lockfiles still tripped the gate and re-ran a full
`uv sync` for every affected directory.

Gate on `uv lock --check` instead, which is the authoritative staleness
test and covers transitive local path dependencies (verified: editing
unstract/core/pyproject.toml marks backend/uv.lock stale). Directories whose
lockfiles are already correct are now a real no-op.

Also swap `uv sync` for `uv lock`. The workflow only ever commits uv.lock,
so building a virtualenv and installing every package was wasted work and
the largest source of failures in this job.

Add a per-branch concurrency group. Two pushes in quick succession started
overlapping runs that raced each other's auto-commit, and the loser failed
the job with "failed to push some refs" — 3 of the last 30 runs.

Auto-commit behaviour is unchanged.
@sonarqubecloud

Copy link
Copy Markdown

@greptile-apps

greptile-apps Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Makes lockfile automation skip projects whose lockfiles already pass uv lock --check, replacing environment installation with lock-only regeneration.

  • Adds per-head-branch workflow concurrency and cancels superseded runs.
  • Runs lockfile checks and regeneration inside directory-scoped subshells.
  • Removes the comparison against origin/main and its supporting local-dependency traversal.

Confidence Score: 5/5

The PR appears safe to merge, with no concrete blocking or independently actionable non-blocking issue identified.

The revised script preserves failed-lock propagation, skips only when uv reports the lockfile current, and reruns lock generation on the newest branch state when overlapping workflow runs are cancelled.

Important Files Changed

Filename Overview
.github/workflows/uv-lock-automation.yaml Adds branch-scoped cancellation to prevent overlapping lockfile auto-commit runs; no actionable defect was established.
docker/scripts/uv-lock-gen/uv-lock.sh Replaces Git-diff-based gating and full environment synchronization with authoritative lock checks and lock-only regeneration while preserving failure propagation.

Reviews (1): Last reviewed commit: "UN-1929 [FIX] Skip uv locking when lockf..." | Re-trigger Greptile

@github-actions

Copy link
Copy Markdown
Contributor

Unstract test results

Per-group results

Status Group Tier Passed Failed Errors Skipped Duration (s)
e2e-api-deployment e2e 3 0 0 0 20.7
e2e-coowners e2e 1 0 0 0 1.7
e2e-etl e2e 1 0 0 0 8.6
e2e-login e2e 2 0 0 0 1.2
e2e-prompt-studio e2e 1 0 0 0 4.7
e2e-smoke e2e 2 0 0 0 1.4
e2e-workflow e2e 1 0 0 0 16.4
integration-backend integration 267 0 0 26 46.6
integration-connectors integration 1 0 0 7 8.1
integration-workers integration 140 0 0 1 51.1
unit-backend unit 998 0 0 1 28.2
unit-connectors unit 63 0 0 0 9.4
unit-core unit 33 0 0 0 1.0
unit-platform-service unit 15 0 0 0 2.1
unit-rig unit 117 0 0 0 3.8
unit-sdk1 unit 518 0 0 0 17.1
unit-workers unit 1346 0 0 1 96.1
TOTAL 3509 0 0 36 318.1

Critical paths

⚠️ Critical paths not yet covered

  • workflow-execution-fan-out — Multi-file workflow execution fans out to file-processing workers and rejoins. (declared coverage: no groups declared)
✅ Covered critical paths
  • auth-login — covered by e2e-login
  • adapter-register-llm — covered by integration-backend
  • workflow-author — covered by integration-backend
  • co-owner-manage — covered by integration-backend, e2e-coowners
  • workflow-create-execute — covered by e2e-workflow
  • api-deployment-provision — covered by integration-backend
  • api-deployment-auth — covered by integration-backend
  • api-deployment-run — covered by e2e-api-deployment
  • mcp-server-auth — covered by integration-backend
  • mcp-platform-auth — covered by integration-backend
  • prompt-studio-author — covered by integration-backend
  • prompt-studio-fetch-response — covered by e2e-prompt-studio
  • connector-register-test — covered by integration-backend
  • pipeline-etl-execute — covered by e2e-etl
  • usage-aggregate-read — covered by integration-backend
  • usage-token-tracking — covered by e2e-api-deployment
  • callback-result-delivery — covered by e2e-api-deployment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants