Skip to content

fix(deps): exclude argcomplete 3.7.1 on Python < 3.10, gate releases on tests - #1127

Merged
jim-fal merged 5 commits into
mainfrom
jim/pin-argcomplete-py39
Aug 5, 2026
Merged

jim-fal merged 5 commits into
mainfrom
jim/pin-argcomplete-py39

Conversation

@jim-fal

@jim-fal jim-fal commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Problem

argcomplete 3.7.1, released 2026-08-04 15:03 UTC, annotates

choices: Final[Mapping[str, str | bytes]]

in a class body (completers.py:35) while still declaring requires-python >=3.8. PEP 604 X | Y at runtime needs 3.10, so on 3.9 the import raises:

TypeError: unsupported operand type(s) for |: 'type' and 'type'

On 3.8 it fails one line earlier still, at the PEP 585 -> Iterable[str] return annotation.

fal.cli.parser imports argcomplete at module scope, so this takes down the entire CLI import chain — fal.cli.parserfal.cli.runnersfal.cli.appsfal.cli.mainfal.cli.

Blast radius

Not only CI. Every fal version already on PyPI is affected, because the open argcomplete>=3.1.0,<4 bound resolves to whatever is newest at install time:

Install today argcomplete resolved fal --help
fal==1.79.0 on py3.9 3.7.1 crashes
fal==1.78.3 on py3.9 (predates the break by a month) 3.7.1 crashes
fal==1.79.0 on py3.10 3.7.1 ok

In CI, unit (py 3.8) and unit (py 3.9) went red on every PR that has run since the release — #1119, #1125, #1126 — each with 48 collection errors across tests/unit/cli/* and tests/unit/console/test_encoding.py. main was last green at 06:19 UTC, before the release, and goes red on its 05:30 UTC cron.

Upstream

Introduced by kislyuk/argcomplete#554 ("100% type coverage"), a typing sweep adding PEP 585/604 annotations with no from __future__ import annotations. It merged 49 seconds after 0d1cb9ad ("Drop EOL Pythons from CI") removed 3.8/3.9 from the matrix — those jobs had been failing on the PR. requires-python was never updated to match, which is what leaves pip installing it on 3.8/3.9.

Tracked at kislyuk/argcomplete#559; kislyuk/argcomplete#558 ("Require Python 3.10+") is open and would resolve it by formalizing the drop.

Changes

1. Exclude the bad version on Python < 3.10projects/fal/pyproject.toml

"argcomplete>=3.1.0,<4; python_version >= '3.10'",
"argcomplete>=3.1.0,!=3.7.1,<4; python_version < '3.10'",

Version-specific rather than a <3.7.1 ceiling, so it lapses cleanly when a fixed release ships and never holds 3.8/3.9 back further than it has to. Python 3.10+ keeps tracking latest.

2. Gate the PyPI release on a fresh unit-test runrelease.yaml, fal-unit-tests.yml

fal 1.79.0 was published at 18:46 UTC from 1604f774, whose last unit run was green at 06:19 — 12.5 hours earlier, and 9 hours before argcomplete 3.7.1 existed. release.yaml runs build → pypi-publish and neither runs nor checks tests, so a release inherits whatever green happened to be on the commit, however old.

Checking the commit's stored status would not have helped here; it was green. Because the dependency bounds are open, the installed tree is a function of when the job runs rather than of the commit, so only re-resolving at release time surfaces it.

fal-unit-tests.yml gains a workflow_call trigger, and pypi-publish now requires it:

  unit-tests:
    if: ${{ needs.build.outputs.name == 'fal' }}
    needs: build
    uses: ./.github/workflows/fal-unit-tests.yml

  pypi-publish:
    needs: [build, unit-tests]
    if: >-
      ${{ !cancelled()
        && needs.build.result == 'success'
        && (needs['unit-tests'].result == 'success' || needs['unit-tests'].result == 'skipped') }}

The gate is scoped to the fal project. isolate_proto and fal_client releases skip it, and pypi-publish tolerates a skipped result so those are unaffected — without that, a skipped dependency would skip the publish too.

Trade-off worth naming: a required gate can block a release when something unrelated fails. The unit suite is the fastest reliable signal available here — e2e and integration are chronically red for unrelated credential reasons and are deliberately not part of the gate.

3. An escape hatch for urgent releasesrelease.yaml

A required gate needs a bypass, or the first urgent release fights it. skip_tests is a workflow_dispatch input defaulting to false:

gh workflow run release.yaml -f tag=fal_v1.79.1 -f skip_tests=true

It exists only on the manual path. A release: published event leaves inputs empty, so the automatic path stays gated with no way to bypass it — an emergency release has to be a deliberate dispatch, and the run records the actor and the input value.

One subtlety worth flagging for review, because it is invisible when wrong. The condition uses inputs.skip_tests, not github.event.inputs.skip_tests. The latter is always a string, and every non-empty string is truthy in an expression, so !github.event.inputs.skip_tests evaluates !"false"false and would skip the tests on every dispatch — the gate silently off, looking fine. Measured on both forms:

Trigger !inputs.skip_tests (used) !github.event.inputs.skip_tests (trap)
skip_tests=false true → tests run falsetests skipped
skip_tests=true false → skipped false → skipped
push / release event true → tests run true → tests run

The forms agree everywhere except the most common dispatch case, which is what would have hidden it.

How to test

Reproduce the break, then confirm the pin:

uv venv /tmp/ac39 --python 3.9
uv pip install --python /tmp/ac39/bin/python "argcomplete==3.7.1"
/tmp/ac39/bin/python -c "import argcomplete"   # TypeError

uv pip install --python /tmp/ac39/bin/python -e "projects/fal[test]"
/tmp/ac39/bin/python -c "import fal.cli.parser"
/tmp/ac39/bin/python -m pytest projects/fal/tests/unit/cli projects/fal/tests/unit/console

Confirm a clean runtime install, the path a user takes:

uv venv /tmp/smoke39 --python 3.9
uv pip install --python /tmp/smoke39/bin/python ./projects/fal   # runtime deps only
/tmp/smoke39/bin/fal --help

CI signal to watch: unit (ubuntu-latest, py 3.8, pydantic==1.10.18) and unit (ubuntu-latest, py 3.9, pydantic==2.13.3) must go green. The release gate cannot run on a PR — it is exercised only by a release event — so it was validated statically, below.

Verified in this session:

Check Result
import argcomplete on py3.9 with 3.7.1 TypeError: unsupported operand type(s) for | — reproduced
import argcomplete on py3.9 with 3.7.0 ok
Resolution on py3.9 after the pin argcomplete 3.7.0
Resolution on py3.12 after the pin argcomplete 3.7.1 — latest, unaffected
pytest tests/unit/cli tests/unit/console on py3.9 247 passed (was 48 collection errors)
Same, merged into #1126's head 248 passed, clean merge
Full tests/unit on py3.9 907 passed; 12 errors all in test_file_sync.py from absent local credentials (401), which CI supplies
Clean runtime install on py3.8 and py3.9, unconstrained fal --help crashes
Clean runtime install on py3.8 and py3.9, with this branch resolves 3.7.0, fal --help ok
Published fal==1.79.0 + argcomplete!=3.7.1 on py3.9 fal --help ok — an upstream yank alone would repair released versions
Wheel built from this branch — metadata argcomplete!=3.7.1,<4,>=3.1.0; python_version < "3.10" present
That wheel installed fresh on py3.8 and py3.9 resolves argcomplete 3.7.0, imports ok, fal --help ok
actionlint on release.yaml and fal-unit-tests.yml clean (the one repo-wide finding is a pre-existing set-output deprecation in container.yml)

Release gate, executed

The gate cannot run on a PR, so its job graph was reproduced exactly — same if expression, publish step replaced by an echo — and run on a throwaway branch, since deleted:

Scenario unit-tests pypi-publish Expected
fal release, tests pass success ran ✅ publishes
fal release, tests fail failure skipped ✅ gate holds
isolate_proto release skipped ran ✅ unaffected
Real uses: ./.github/workflows/fal-unit-tests.yml 8/8 jobs success workflow_call is valid
Dispatch, skip_tests=false success ran ✅ default stays gated
Dispatch, skip_tests=true skipped ran ✅ bypass works
Push / no inputs (stands in for release: published) success ran ✅ automatic path gated

The third row is the one that would silently break other projects' releases if the condition were wrong; the fourth confirms the reusable-workflow call resolves; the last three cover the bypass, including that it cannot be reached from the automatic path.

Not verified: the py3.8 leg of the unit suite locally — no 3.8 interpreter with test extras to hand; CI covers it directly, and the py3.8 install path is verified above. The gate has not run against a real release event, which would require publishing; the simulation above covers its logic and the reusable call.

Follow-ups, not in this PR

  • A scheduled canary against the published package. Nothing looks at PyPI, so an upstream break of an already-released version is invisible until someone reports it. Opening separately.
  • release-container still only needs: build, so a container can ship from a build the gate would have blocked. Left alone here to keep this change to the PyPI path; worth extending.
  • An upstream yank of argcomplete 3.7.1 would repair every already-published fal version with no release from us — worth requesting on kislyuk/argcomplete#559. This PR is the path that does not depend on upstream acting.
  • Whether fal should keep supporting 3.8/3.9. Both are EOL and upstream argcomplete is dropping them. A product decision, separate from this fix.

Note

e2e and integration failures on this repo are a separate, pre-existing matter — they fail on main too, with Insufficient permissions from the CI service account, and have for 8+ days. Untouched here.

🤖 Generated with Claude Code


Note

Medium Risk
Changes release gating and install-time dependency resolution for all fal users on 3.8/3.9; incorrect workflow conditions could block publishes or skip tests on dispatch.

Overview
Pins argcomplete so Python 3.8/3.9 never resolve 3.7.1 (PEP 604 annotations break CLI import), while 3.10+ keep the existing upper bound.

Release automation now re-runs fal unit tests with fresh dependency resolution before PyPI publish and container push; fal-unit-tests.yml is reusable via workflow_call. Manual releases get an optional skip_tests bypass using inputs.skip_tests (not stringly github.event.inputs).

Reviewed by Cursor Bugbot for commit f5bc141. Bugbot is set up for automated code reviews on this repo. Configure here.

argcomplete 3.7.1 (released 2026-08-04) annotates `choices: Final[Mapping[str,
str | bytes]]` in a class body while still declaring `requires-python >=3.8`, so
importing it raises `TypeError: unsupported operand type(s) for |` on 3.8 and
3.9. `fal.cli.parser` imports argcomplete at module scope, so the whole CLI —
and every `tests/unit/cli` and `tests/unit/console` module — fails to import.

The `unit (py 3.8)` and `unit (py 3.9)` jobs went red on every open PR within
hours of the release, with 48 collection errors each.

Upstream is aware (kislyuk/argcomplete#559, fix in kislyuk/argcomplete#558) and
may yank the release; the exclusion is version-specific so it lapses cleanly
once a fixed version ships. Python 3.10+ keeps tracking latest.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@socket-security

socket-security Bot commented Aug 4, 2026

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedpypi/​argcomplete@​3.7.097100100100100

View full report

@jim-fal

jim-fal commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

CI confirms it, and isolates it cleanly. Comparing this PR against the two other open PRs on the same base, at the same time:

PR unit (py 3.8) unit (py 3.9) Other failures
#1127 (this) ✅ pass ✅ pass 16
#1119 ❌ fail ❌ fail 16
#1126 ❌ fail ❌ fail 16

The two unit jobs are the only difference. The remaining 16 are the identical set of e2e and integration jobs on all three, failing with UNAUTHENTICATED: Insufficient permissions — they fail on main too and are untouched by this PR.

unit on py 3.10 through 3.14 and windows-2022 stayed green throughout, so the marker does not disturb the versions that were already fine.

@jim-fal
jim-fal marked this pull request as ready for review August 5, 2026 00:25
@jim-fal jim-fal changed the title DRAFT: fix(deps): exclude argcomplete 3.7.1 on Python < 3.10 fix(deps): exclude argcomplete 3.7.1 on Python < 3.10 Aug 5, 2026

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — a minimal, well-verified dependency pin that unblocks CI.

Extended reasoning...

Overview

Single-line change to projects/fal/pyproject.toml: splits the argcomplete dependency constraint into two environment markers, adding !=3.7.1 for python_version < '3.10' while leaving 3.10+ tracking latest. No runtime/application code is touched.

Security risks

None. This is a version-exclusion pin on a build/lint-time CLI completion dependency, not a security-sensitive path (no auth, crypto, or permission logic involved).

Level of scrutiny

Low. This is a mechanical, reversible dependency fix responding to a concrete, reproduced upstream breakage (PEP 604 syntax incompatible with the package's own declared requires-python). The PR includes clear reproduction steps and verification output (247 previously-erroring tests now passing on py3.9), and a maintainer (jim-fal) independently confirmed via CI comparison against sibling PRs that the two unit jobs are the only jobs affected and now pass, with no regression on py3.10–3.14 or Windows.

Other factors

The scoped !=3.7.1 marker (rather than a blanket ceiling) is the right shape — it self-resolves once upstream ships a fix or yanks the bad release, and the inline comment documents the upstream issue for future readers. No outstanding review comments to address.

jim-fal and others added 2 commits August 4, 2026 17:49
The unit suite installs `projects/fal[test]` against a checkout, so it never
exercises a runtime-only dependency resolution, and nothing in CI looks at what
is already published on PyPI. argcomplete 3.7.1 broke both: every released fal
version stopped importing on 3.8/3.9 with no commit to this repo, and fal 1.79.0
was published 3h43m after the breakage began.

Two jobs, both across the full supported matrix:

- `source install` runs on pyproject changes and installs `./projects/fal` with
  runtime dependencies only, then starts the CLI. Catches a bad resolution
  before it ships.
- `published fal` is scheduled-only and installs the released package straight
  from PyPI. This is the one that catches an upstream break of a version already
  in users' hands, which no commit-triggered job can see.

Verified against the incident: on 3.8 and 3.9 both jobs fail on unconstrained
argcomplete (`fal --help` raises TypeError) and pass with the exclusion in this
branch.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
fal 1.79.0 was published from a commit whose last unit run was green 12.5 hours
earlier. In between, argcomplete 3.7.1 broke the CLI on 3.8/3.9, so the release
shipped an artifact that could not start on two supported interpreters. Nothing
in release.yaml runs or checks tests, so there was nothing to stop it.

Checking the commit's stored status would not have helped -- it was green. The
dependency bounds are open, so the installed tree is a function of when the job
runs, not of the commit. Only re-resolving at release time surfaces this.

fal-unit-tests.yml gains a `workflow_call` trigger; release.yaml calls it after
build and pypi-publish now requires it. The gate is scoped to the fal project,
and pypi-publish tolerates a skipped result so isolate_proto and fal_client
releases are unaffected.

Verified: actionlint clean on both changed workflows, and the publish condition
resolves correctly for tests-pass, tests-fail, non-fal-skipped, build-failed and
cancelled.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@jim-fal jim-fal changed the title fix(deps): exclude argcomplete 3.7.1 on Python < 3.10 fix(deps): exclude argcomplete 3.7.1 on Python < 3.10, gate releases on tests Aug 5, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 02e0000. Configure here.

Comment thread .github/workflows/release.yaml
jim-fal and others added 2 commits August 4, 2026 23:48
A required gate needs an escape hatch, or the first urgent release fights it.

`skip_tests` is a workflow_dispatch input defaulting to false. It exists only on
the manual path: `release: published` leaves `inputs` empty, so the automatic
path stays gated with no way to bypass. An emergency release becomes a
deliberate, attributable action, and the run records actor and input.

The condition uses `inputs.skip_tests`, not `github.event.inputs.skip_tests`.
The latter is always a string and every non-empty string is truthy, so the buggy
form evaluates `!"false"` to false and would disable the gate on every dispatch,
silently. Confirmed on a throwaway branch -- with skip_tests=false the correct
form yields true (tests run) and the buggy form yields false (tests skipped);
the two agree in every other case, which is what would have hidden it.

Verified: push/no-inputs -> tests run, publish runs; dispatch skip_tests=false
-> tests run, publish runs; dispatch skip_tests=true -> tests skipped, publish
runs.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
release-container only needed `build`, so it ran in parallel with the suite and
could push ghcr.io/fal-ai/fal:latest from a build the gate had just blocked from
PyPI -- a gate with a hole in it. Raised by Bugbot on the PR.

Same condition as pypi-publish, plus the pre-existing fal-only guard folded into
it. The `skipped` tolerance covers the skip_tests bypass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@jim-fal
jim-fal merged commit ea19abf into main Aug 5, 2026
18 of 34 checks passed
@jim-fal
jim-fal deleted the jim/pin-argcomplete-py39 branch August 5, 2026 16:35
jim-fal added a commit that referenced this pull request Aug 6, 2026
The unit gate added in #1127 installs the source tree
(`pip install -e projects/fal`), so nothing in the release run ever
installs the wheel `pypi-publish` uploads. A wheel is a filtered copy of
the source plus generated metadata, so a module missing from the
packaging config, a data file not included, a broken console-script
entry point, or a dependency marker absent from METADATA all leave the
suite green and ship anyway.

Add a `wheel-smoke` job between `unit-tests` and publish: download the
built wheel, install it into a clean runner with no checkout on disk,
import the package and run `fal --version` / `fal --help`. Matrix covers
3.8 and 3.9, where the dependency markers diverge from 3.10+, plus 3.13.
`pypi-publish` and `release-container` now require it on the same
success-or-skipped terms as `unit-tests`.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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