Skip to content

fix(api-sync): make --apply idempotent per (class, field/enum value) - #39

Merged
ericviana merged 1 commit into
mainfrom
eric/fix-apply-double-insert
Aug 4, 2026
Merged

ericviana merged 1 commit into
mainfrom
eric/fix-apply-double-insert

Conversation

@ericviana

Copy link
Copy Markdown
Member

Summary

  • reconcileTypes() and reconcileEnums() computed missing fields/enum cases per
    schema (or per map entry), not per underlying SDK class. A multi-schema
    spec-map.json entry sharing one class (e.g. spec: ["PayinOut", "CreatePayinOut"]
    both pointing at PayinTrackingPayment), or two separate entries mapping the same
    class (TrackingTransaction reused across the payin and payout tracking_transaction
    paths, or LimitIncreaseRequestSupportingDocumentType mapped from two different
    schemas), could independently report the exact same missing (class, field) or
    (class, value) gap once per schema/entry that lacked it.
  • --apply then inserted the same promoted constructor property / fromArray line /
    toArray line, or the same enum case, twice into one file in a single run --
    corrupting the source with duplicate constructor parameters. Hit this for real while
    wiring up the new coverage checks in feat(api-sync): enum and nested-object coverage checks, delivery manifest verification #38 (had to revert and apply those fields by
    hand instead of trusting --apply).
  • Fix: two small dedupe indexes, $scheduledInsertions (keyed class|field) in
    reconcileTypes() and $scheduledCases (keyed class|value) in reconcileEnums().
    Each function now emits at most one applicable change per (class, field-or-value)
    per run, regardless of how many schemas or map entries independently found it
    missing. A second --apply over already-patched source is byte-identical (verified
    via the existing determinism-proof pattern, run locally against the real repo).

Regression tests

Three new tests in tests/ApiSync/ApiSyncTest.php, each reproducing one trigger shape
of the exact bug and verified to fail (2 applicable entries instead of 1) against the
pre-fix code, pass after:

  • a_multi_schema_entry_missing_the_same_field_on_both_schemas_is_only_scheduled_once
  • two_separate_entries_mapping_the_same_class_missing_the_same_field_are_only_scheduled_once
  • two_enum_entries_mapping_the_same_class_missing_the_same_value_are_only_scheduled_once

Each also applies the (deduped) change and asserts the field/case appears in the
patched file exactly once, then re-runs reconciliation to confirm nothing is left
pending.

No version bump

Pure tooling correctness fix, not a change to the published SDK's public surface.

Test plan

  • composer run test -- 121 passed (118 pre-existing + 3 new), 743 assertions
  • New tests confirmed to fail pre-fix (git stash on scripts/api-sync.php only)
    and pass post-fix
  • composer run lint:check -- clean
  • php scripts/api-sync.php --check against the real repo's spec-snapshot -- exit 0
  • Map validity check -- OK
  • Determinism proof (apply into two scratch copies of the real repo, diff
    byte-identical) -- OK
  • php scripts/contract-check.php -- OK

https://claude.ai/code/session_01F1stiNzuNtJXoXtiW9ZCbs

… enum value)

reconcileTypes()/reconcileEnums() computed missing fields/enum cases per schema, not
per underlying SDK class. A multi-schema spec-map.json entry (e.g. spec: ["PayinOut",
"CreatePayinOut"] sharing one class) or two separate entries mapping the same class
(TrackingTransaction reused across the payin and payout tracking_transaction paths;
LimitIncreaseRequestSupportingDocumentType mapped from two different schemas) could
independently report the exact same missing (class, field) or (class, value) gap once
per schema/entry that lacked it. --apply then inserted the same promoted property/
fromArray line/toArray line, or the same enum case, twice into one file in a single
run -- corrupting the source with duplicate constructor parameters on the first real
multi-schema drift.

Fixes it by deduplicating at the point applicable changes are generated: a
$scheduledInsertions index in reconcileTypes() (keyed by class|field) and a
$scheduledCases index in reconcileEnums() (keyed by class|value) make each function
emit at most one applicable change per (class, field-or-value) per run, regardless of
how many schemas or map entries independently found it missing. A second --apply run
over already-patched source is byte-identical (verified via the existing determinism
proof).

Adds three regression tests reproducing the exact double-apply: a multi-schema single
entry, two separate entries mapping the same class, and the enum-side equivalent.
All three fail (2 applicable instead of 1) against the pre-fix code and pass after.

No version bump -- pure tooling correctness fix.

Claude-Session: https://claude.ai/code/session_01F1stiNzuNtJXoXtiW9ZCbs
@BernardoSM

Copy link
Copy Markdown
Contributor

✅ Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
✅ Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@ericviana
ericviana merged commit 36f3f90 into main Aug 4, 2026
6 checks passed
@ericviana
ericviana deleted the eric/fix-apply-double-insert branch August 4, 2026 13:07
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