Skip to content

Adopt shared SDK and reconcile ambiguous uploads - #2

Merged
SahirVhora merged 5 commits into
mainfrom
codex/sdk-adoption-object-sync
Jul 20, 2026
Merged

Adopt shared SDK and reconcile ambiguous uploads#2
SahirVhora merged 5 commits into
mainfrom
codex/sdk-adoption-object-sync

Conversation

@SahirVhora

@SahirVhora SahirVhora commented Jul 4, 2026

Copy link
Copy Markdown
Owner

What changed

  • adopts the shared sapsf-shared client and authentication contract
  • uses the shared OData escaping and pagination behavior
  • catches the new AmbiguousWriteError for create operations
  • reconciles an uncertain create by querying externalCode instead of replaying POST
  • records reconciled creates as verified successes and unresolved outcomes as failures
  • adds regression coverage for both reconciliation paths

Why

A SuccessFactors create can commit successfully while its response is lost. Replaying that POST can produce a duplicate or a misleading failure. The uploader now checks target state and safely continues dependent hierarchy uploads when the original object exists.

Validation

  • pytest: 104 passed
  • scoped Ruff checks: passed
  • git diff --check: passed

Merge order

Merge SahirVhora/sapsf-shared#2 first. This PR imports AmbiguousWriteError introduced by that upstream change.

Note: the existing OAuth company-ID review comment predates this safety fix and remains separate follow-up work.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1459f3d194

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/auth_handler.py
token_url = os.getenv(f"{env_prefix}_TOKEN_URL", "")
if not client_id or not client_secret or not token_url:
company_id = os.getenv(f"{env_prefix}_COMPANY_ID", os.getenv("SF_COMPANY_ID", ""))
if not client_id or not client_secret or not token_url or not company_id:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Wire OAuth company ID before requiring it

With this new check, OAuth clients built from the web UI cannot be created: the OAuth sections in web_ui/templates/index.html only collect client id/secret/token URL, and _test_one_connection/process only export those SF_* env vars before calling build_sf_client. Because company_id is only read from SF_<env>_COMPANY_ID/SF_COMPANY_ID, selecting OAuth in the UI fails with this AuthError unless a server-side env var happens to be pre-set; either add/wire the company-id field for OAuth or pass it from the submitted config before this validation.

Useful? React with 👍 / 👎.

@SahirVhora SahirVhora closed this Jul 4, 2026
@SahirVhora SahirVhora reopened this Jul 4, 2026
@SahirVhora
SahirVhora force-pushed the codex/sdk-adoption-object-sync branch from 3e5f148 to bb8a004 Compare July 4, 2026 12:18
@SahirVhora SahirVhora changed the title refactor: adopt sapsf-shared SDK client Adopt shared SDK and reconcile ambiguous uploads Jul 10, 2026
@SahirVhora
SahirVhora merged commit f0c624a into main Jul 20, 2026
1 check passed
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