Skip to content

Regenerate Swift bindings and commit them on every release - #26

Merged
dubadub merged 1 commit into
mainfrom
fix/regenerate-swift-bindings-0.6.0
Sep 2, 2026
Merged

dubadub merged 1 commit into
mainfrom
fix/regenerate-swift-bindings-0.6.0

Conversation

@dubadub

@dubadub dubadub commented Sep 2, 2026

Copy link
Copy Markdown
Member

The SPM package compiles swift/Sources/CooklangSync straight out of the
repo, but the release workflow only ever generated the bindings into the
release zip and committed Package.swift. The checked-in bindings had not
been regenerated since v0.4.10, so every consumer since v0.4.11 has been
compiling an enum that no longer matches the xcframework it links against.

SyncError is a flat_error, so the variant is carried across FFI as a
positional index. The stale file still declares ReqwestWirhMiddlewareError
at index 9 — a variant Rust dropped in 04a0560 — which shifts everything
after it by one:

Rust ChannelSendError    (9)  -> Swift ReqwestWirhMiddlewareError
Rust ConnectionInitError (10) -> Swift ChannelSendError
Rust Unauthorized        (11) -> Swift ConnectionInitError
Rust PaymentRequired     (12) -> Swift Unauthorized

On 0.6.0 that last row is the damaging one: the 402 added in f0a517f
surfaces to iOS as Unauthorized, i.e. "your session expired", which is
exactly the wrong route for a product gate.

Two changes:

  • Replace the stale bindings with the ones the v0.6.0 release actually
    generated (byte-identical to cooklang-sync-ios.zip). The only
    difference from the checked-in file is the SyncError enum; renamed to
    CooklangSync.swift to match the generator's output filename so CI can
    overwrite it in place.
  • Have prepare-release pull the generated bindings from build-ios and
    commit them alongside Package.swift, so the tag can never again point
    at bindings that disagree with its xcframework.

The SPM package compiles `swift/Sources/CooklangSync` straight out of the
repo, but the release workflow only ever generated the bindings into the
release zip and committed `Package.swift`. The checked-in bindings had not
been regenerated since v0.4.10, so every consumer since v0.4.11 has been
compiling an enum that no longer matches the xcframework it links against.

`SyncError` is a `flat_error`, so the variant is carried across FFI as a
positional index. The stale file still declares `ReqwestWirhMiddlewareError`
at index 9 — a variant Rust dropped in 04a0560 — which shifts everything
after it by one:

    Rust ChannelSendError    (9)  -> Swift ReqwestWirhMiddlewareError
    Rust ConnectionInitError (10) -> Swift ChannelSendError
    Rust Unauthorized        (11) -> Swift ConnectionInitError
    Rust PaymentRequired     (12) -> Swift Unauthorized

On 0.6.0 that last row is the damaging one: the 402 added in f0a517f
surfaces to iOS as `Unauthorized`, i.e. "your session expired", which is
exactly the wrong route for a product gate.

Two changes:

- Replace the stale bindings with the ones the v0.6.0 release actually
  generated (byte-identical to `cooklang-sync-ios.zip`). The only
  difference from the checked-in file is the `SyncError` enum; renamed to
  `CooklangSync.swift` to match the generator's output filename so CI can
  overwrite it in place.
- Have `prepare-release` pull the generated bindings from `build-ios` and
  commit them alongside `Package.swift`, so the tag can never again point
  at bindings that disagree with its xcframework.
@claude

claude Bot commented Sep 2, 2026

Copy link
Copy Markdown

Review

Nice root-cause diagnosis — tracing the Unauthorized/PaymentRequired mixup back to the dropped ReqwestWirhMiddlewareError variant and confirming the fix against client/src/errors.rs is exactly the kind of forensic work this bug needed. I checked the new CooklangSync.swift enum (both the read and write sides of FfiConverterTypeSyncError) against the Rust SyncError definition — the indices now match 1:1 (IoErrorBatchDownloadError, 1–17, no gaps), so the immediate 0.6.0 mis-routing bug is fixed correctly.

Bug: scripts/verify_ios_build.sh still references the old filename

The rename from CooklangSyncClient.swiftCooklangSync.swift isn't reflected in scripts/verify_ios_build.sh:41-46, which hardcodes $SWIFT_SOURCES/CooklangSyncClient.swift. That file no longer exists post-rename, so this script will now always print ✗ Swift bindings not found! and exit 1.

It's not wired into any GitHub workflow today (I grepped .github/workflows/*.yml and found no reference), so it won't break CI, but it's a real regression for anyone running it locally to sanity-check an iOS build. Worth a follow-up to point it at the new filename — or better, glob for *.swift so it doesn't hardcode a specific generated filename again.

CI logic (release.yml)

  • The Clear checked-in Swift bindingsSync generated Swift bindingsVerify bindings landed sequence in prepare-release is well ordered: the zero-file check happens before the commit step, so a broken/empty artifact download fails the job instead of silently committing nothing (or committing a stale deletion).
  • Removing old files before restoring from the artifact is a nice touch — it means a future rename of the generated file (like this PR's) can't leave an orphaned stale file behind, which is exactly the class of bug this PR is fixing.
  • One gap: the new bindings-sync steps only run in prepare-release, which is skipped entirely when dry_run: true. So this fix's own CI logic can't be exercised by a dry run — only a real release (or manual inspection, like the diff review here) verifies it works. Given this bug came from exactly this kind of untested release-path drift, it might be worth a comment in the workflow noting that a dry run doesn't cover this path, so nobody assumes dry-run green means the bindings sync is verified.

Minor

  • The renamed file is a pure mechanical diff (regenerated bindings, byte-identical to the release zip per the PR description) — nothing else to flag there.
  • No test coverage changes, which is reasonable here since the bug is in generated code / CI wiring rather than logic that unit tests would naturally cover; the workflow-level verification steps are the appropriate substitute.

Overall this looks correct and directly addresses the root cause rather than patching the symptom. Recommend fixing the verify_ios_build.sh filename reference before/after merging so it doesn't bite the next person who runs it locally.

@dubadub
dubadub merged commit 004e2fc into main Sep 2, 2026
7 checks passed
@dubadub
dubadub deleted the fix/regenerate-swift-bindings-0.6.0 branch September 2, 2026 13:33
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.

1 participant